On 2023/7/1 5:59, Jaegeuk Kim wrote:
On 06/28, Jaegeuk Kim wrote:
On 06/28, Chao Yu wrote:
On 2023/6/28 16:08, Jaegeuk Kim wrote:
Thread #1:
[122554.641906][ T92] f2fs_getxattr+0xd4/0x5fc -> waiting for f2fs_down_read(&F2FS_I(inode)->i_xattr_sem);
[122554.641927][ T92] __f2fs_get_acl+0x50/0x284 [122554.641948][ T92] f2fs_init_acl+0x84/0x54c [122554.641969][ T92] f2fs_init_inode_metadata+0x460/0x5f0 [122554.641990][ T92] f2fs_add_inline_entry+0x11c/0x350 -> Locked dir->inode_page by f2fs_get_node_page()
[122554.642009][ T92] f2fs_do_add_link+0x100/0x1e4 [122554.642025][ T92] f2fs_create+0xf4/0x22c [122554.642047][ T92] vfs_create+0x130/0x1f4
Thread #2:
[123996.386358][ T92] __get_node_page+0x8c/0x504 -> waiting for dir->inode_page lock
[123996.386383][ T92] read_all_xattrs+0x11c/0x1f4 [123996.386405][ T92] __f2fs_setxattr+0xcc/0x528 [123996.386424][ T92] f2fs_setxattr+0x158/0x1f4 -> f2fs_down_write(&F2FS_I(inode)->i_xattr_sem);
[123996.386443][ T92] __f2fs_set_acl+0x328/0x430 [123996.386618][ T92] f2fs_set_acl+0x38/0x50 [123996.386642][ T92] posix_acl_chmod+0xc8/0x1c8 [123996.386669][ T92] f2fs_setattr+0x5e0/0x6bc [123996.386689][ T92] notify_change+0x4d8/0x580 [123996.386717][ T92] chmod_common+0xd8/0x184 [123996.386748][ T92] do_fchmodat+0x60/0x124 [123996.386766][ T92] __arm64_sys_fchmodat+0x28/0x3c
Back to the race condition, my question is why we can chmod on inode before it has been created?
This is touching the directory.
Chao,
Do you have other concern? Otherwise, I'll include this into the next pull request.
Jaegeuk,
Sorry for late reply, I was misled by "dir->inode_page" and "inode" words in commit message, it should be "dir->inode_page" and "dir_inode".
Anyway, the patch looks good to me.
Reviewed-by: Chao Yu chao@kernel.org
Thanks,
Thanks,
Thanks,
Fixes: 27161f13e3c3 "f2fs: avoid race in between read xattr & write xattr" Cc: stable@vger.kernel.org Signed-off-by: Jaegeuk Kim jaegeuk@kernel.org
fs/f2fs/dir.c | 9 ++++++++- fs/f2fs/xattr.c | 6 ++++-- 2 files changed, 12 insertions(+), 3 deletions(-)
diff --git a/fs/f2fs/dir.c b/fs/f2fs/dir.c index 887e55988450..d635c58cf5a3 100644 --- a/fs/f2fs/dir.c +++ b/fs/f2fs/dir.c @@ -775,8 +775,15 @@ int f2fs_add_dentry(struct inode *dir, const struct f2fs_filename *fname, { int err = -EAGAIN;
- if (f2fs_has_inline_dentry(dir))
- if (f2fs_has_inline_dentry(dir)) {
/** Should get i_xattr_sem to keep the lock order:* i_xattr_sem -> inode_page lock used by f2fs_setxattr.*/f2fs_down_read(&F2FS_I(dir)->i_xattr_sem); err = f2fs_add_inline_entry(dir, fname, inode, ino, mode);f2fs_up_read(&F2FS_I(dir)->i_xattr_sem);- } if (err == -EAGAIN) err = f2fs_add_regular_entry(dir, fname, inode, ino, mode);
diff --git a/fs/f2fs/xattr.c b/fs/f2fs/xattr.c index 213805d3592c..476b186b90a6 100644 --- a/fs/f2fs/xattr.c +++ b/fs/f2fs/xattr.c @@ -528,10 +528,12 @@ int f2fs_getxattr(struct inode *inode, int index, const char *name, if (len > F2FS_NAME_LEN) return -ERANGE;
- f2fs_down_read(&F2FS_I(inode)->i_xattr_sem);
- if (!ipage)
f2fs_down_read(&F2FS_I(inode)->i_xattr_sem);
- f2fs_up_read(&F2FS_I(inode)->i_xattr_sem);
- if (!ipage)
f2fs_up_read(&F2FS_I(inode)->i_xattr_sem);
Linux-f2fs-devel mailing list Linux-f2fs-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/linux-f2fs-devel