[PATCH 3/3] ubifs: ensure only one in-memory xattr inode is created
Hou Tao
houtao1 at huawei.com
Tue Jun 30 09:04:38 EDT 2020
ubifs may create two in-memory inodes for one xattr if
there are concurrent ubifs_xattr_get() and ubifs_xattr_set(),
as show in the following case:
ubifs_xattr_get() ubifs_xattr_set()
// the first created inode A
// fill inode A
ubifs_new_inode()
ubifs_jnl_update()
// mapping xattr name to inum
ubifs_tnc_add_nm()
// add xattr inode node
ubifs_tnc_add()
// find inum through xattr name
ubifs_tnc_lookup_nm()
iget_xattr()
ubifs_iget()
// not found in hash table
// so create a new inode B
// and keep it in hash table
iget_locked()
// find xattr inode node
// fill inode B
ubifs_tnc_lookup
unlock_new_inode
// inode A is also inserted into
// hash table
If we update the xattr value afterwards, only the values in inode A will
be updated. So when we ty to remove the xattr name, and in the same
time get the xattr name, ubifs_xattr_get() may return the stale value
in inode B, as show in the following case:
ubifs_xattr_get() ubifs_xattr_remove()
// get xattr inum
ubifs_tnc_lookup_nm()
// return inode A
iget_xattr()
clear_nlink()
remove_xattr()
iput()
evict()
ubifs_evict_inode()
remove_inode_hash()
// return inode B
// return a stale xattr value
iget_xattr()
Fix it by moving insert_inode_hash() before ubifs_jnl_update(),
but after the initialization of inode is completed, so only
one inode is created for xattr value.
Signed-off-by: Hou Tao <houtao1 at huawei.com>
---
fs/ubifs/xattr.c | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/fs/ubifs/xattr.c b/fs/ubifs/xattr.c
index 82be2c2d2db5..10fcb454bb01 100644
--- a/fs/ubifs/xattr.c
+++ b/fs/ubifs/xattr.c
@@ -133,6 +133,15 @@ static int create_xattr(struct ubifs_info *c, struct inode *host,
inode->i_size = ui->ui_size = size;
ui->data_len = size;
+ /*
+ * Ensure iget_xattr() in ubifs_xattr_get() will find the inode
+ * instead of creating a new one.
+ * The initialization of xattr inode is completed here, so using
+ * insert_inode_hash() instead of insert_inode_locked(). The
+ * latter can lead to iget_xattr() return -ESTALE.
+ */
+ insert_inode_hash(inode);
+
mutex_lock(&host_ui->ui_mutex);
host->i_ctime = current_time(host);
host_ui->xattr_cnt += 1;
@@ -156,7 +165,6 @@ static int create_xattr(struct ubifs_info *c, struct inode *host,
mutex_unlock(&host_ui->ui_mutex);
ubifs_release_budget(c, &req);
- insert_inode_hash(inode);
iput(inode);
return 0;
--
2.25.0.4.g0ad7144999
More information about the linux-mtd
mailing list