fs/ext4/inode.c | 2 ++ 1 file changed, 2 insertions(+)
[Syzbot reported]
WARNING: possible circular locking dependency detected
6.8.0-syzkaller-08951-gfe46a7dd189e #0 Not tainted
------------------------------------------------------
syz-executor545/5275 is trying to acquire lock:
ffff888077730400 (&ea_inode->i_rwsem#8/1){+.+.}-{3:3}, at: inode_lock include/linux/fs.h:793 [inline]
ffff888077730400 (&ea_inode->i_rwsem#8/1){+.+.}-{3:3}, at: ext4_xattr_inode_iget+0x173/0x440 fs/ext4/xattr.c:461
but task is already holding lock:
ffff888077730c88 (&ei->i_data_sem/3){++++}-{3:3}, at: ext4_setattr+0x1ba0/0x29d0 fs/ext4/inode.c:5417
which lock already depends on the new lock.
the existing dependency chain (in reverse order) is:
-> #1 (&ei->i_data_sem/3){++++}-{3:3}:
down_write+0x3a/0x50 kernel/locking/rwsem.c:1579
ext4_update_i_disksize fs/ext4/ext4.h:3383 [inline]
ext4_xattr_inode_write fs/ext4/xattr.c:1446 [inline]
ext4_xattr_inode_lookup_create fs/ext4/xattr.c:1594 [inline]
ext4_xattr_set_entry+0x3a14/0x3cf0 fs/ext4/xattr.c:1719
ext4_xattr_ibody_set+0x126/0x380 fs/ext4/xattr.c:2287
ext4_xattr_set_handle+0x98d/0x1480 fs/ext4/xattr.c:2444
ext4_xattr_set+0x149/0x380 fs/ext4/xattr.c:2558
__vfs_setxattr+0x176/0x1e0 fs/xattr.c:200
__vfs_setxattr_noperm+0x127/0x5e0 fs/xattr.c:234
__vfs_setxattr_locked+0x182/0x260 fs/xattr.c:295
vfs_setxattr+0x146/0x350 fs/xattr.c:321
do_setxattr+0x146/0x170 fs/xattr.c:629
setxattr+0x15d/0x180 fs/xattr.c:652
path_setxattr+0x179/0x1e0 fs/xattr.c:671
__do_sys_lsetxattr fs/xattr.c:694 [inline]
__se_sys_lsetxattr fs/xattr.c:690 [inline]
__x64_sys_lsetxattr+0xc1/0x160 fs/xattr.c:690
do_syscall_x64 arch/x86/entry/common.c:52 [inline]
do_syscall_64+0xd5/0x260 arch/x86/entry/common.c:83
entry_SYSCALL_64_after_hwframe+0x6d/0x75
-> #0 (&ea_inode->i_rwsem#8/1){+.+.}-{3:3}:
check_prev_add kernel/locking/lockdep.c:3134 [inline]
check_prevs_add kernel/locking/lockdep.c:3253 [inline]
validate_chain kernel/locking/lockdep.c:3869 [inline]
__lock_acquire+0x2478/0x3b30 kernel/locking/lockdep.c:5137
lock_acquire kernel/locking/lockdep.c:5754 [inline]
lock_acquire+0x1b1/0x540 kernel/locking/lockdep.c:5719
down_write+0x3a/0x50 kernel/locking/rwsem.c:1579
inode_lock include/linux/fs.h:793 [inline]
ext4_xattr_inode_iget+0x173/0x440 fs/ext4/xattr.c:461
ext4_xattr_inode_get+0x16c/0x870 fs/ext4/xattr.c:535
ext4_xattr_move_to_block fs/ext4/xattr.c:2640 [inline]
ext4_xattr_make_inode_space fs/ext4/xattr.c:2742 [inline]
ext4_expand_extra_isize_ea+0x1367/0x1ae0 fs/ext4/xattr.c:2834
__ext4_expand_extra_isize+0x346/0x480 fs/ext4/inode.c:5789
ext4_try_to_expand_extra_isize fs/ext4/inode.c:5832 [inline]
__ext4_mark_inode_dirty+0x55a/0x860 fs/ext4/inode.c:5910
ext4_setattr+0x1c14/0x29d0 fs/ext4/inode.c:5420
notify_change+0x745/0x11c0 fs/attr.c:497
do_truncate+0x15c/0x220 fs/open.c:65
handle_truncate fs/namei.c:3300 [inline]
do_open fs/namei.c:3646 [inline]
path_openat+0x24b9/0x2990 fs/namei.c:3799
do_filp_open+0x1dc/0x430 fs/namei.c:3826
do_sys_openat2+0x17a/0x1e0 fs/open.c:1406
do_sys_open fs/open.c:1421 [inline]
__do_sys_openat fs/open.c:1437 [inline]
__se_sys_openat fs/open.c:1432 [inline]
__x64_sys_openat+0x175/0x210 fs/open.c:1432
do_syscall_x64 arch/x86/entry/common.c:52 [inline]
do_syscall_64+0xd5/0x260 arch/x86/entry/common.c:83
entry_SYSCALL_64_after_hwframe+0x6d/0x75
other info that might help us debug this:
Possible unsafe locking scenario:
CPU0 CPU1
---- ----
lock(&ei->i_data_sem/3);
lock(&ea_inode->i_rwsem#8/1);
lock(&ei->i_data_sem/3);
lock(&ea_inode->i_rwsem#8/1);
*** DEADLOCK ***
[Fix]
According to mark inode dirty context, it does not need to be protected by lock
i_data_sem, and if it is protected by i_data_sem, a deadlock will occur.
Reported-by: syzbot+ee72b9a7aad1e5a77c5c@syzkaller.appspotmail.com
Signed-off-by: Edward Adam Davis <eadavis@qq.com>
---
fs/ext4/inode.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/fs/ext4/inode.c b/fs/ext4/inode.c
index 537803250ca9..d2cbe3dddfab 100644
--- a/fs/ext4/inode.c
+++ b/fs/ext4/inode.c
@@ -5417,6 +5417,7 @@ int ext4_setattr(struct mnt_idmap *idmap, struct dentry *dentry,
down_write(&EXT4_I(inode)->i_data_sem);
old_disksize = EXT4_I(inode)->i_disksize;
EXT4_I(inode)->i_disksize = attr->ia_size;
+ up_write(&EXT4_I(inode)->i_data_sem);
rc = ext4_mark_inode_dirty(handle, inode);
if (!error)
error = rc;
@@ -5425,6 +5426,7 @@ int ext4_setattr(struct mnt_idmap *idmap, struct dentry *dentry,
* with i_disksize to avoid races with writeback code
* running ext4_wb_update_i_disksize().
*/
+ down_write(&EXT4_I(inode)->i_data_sem);
if (!error)
i_size_write(inode, attr->ia_size);
else
--
2.43.0On Thu, Apr 04, 2024 at 09:54:02AM +0800, Edward Adam Davis wrote:
> According to mark inode dirty context, it does not need to be protected by lock
> i_data_sem, and if it is protected by i_data_sem, a deadlock will occur.
The i_data_sem lock is not to protect mark_inode_dirty_context, but to
avoid races with the writeback code, which you can see right before
you added the down_write() line.
More detail about why it is necessary can be found in commit
90e775b71ac4 ("ext4: fix lost truncate due to race with writeback"):
The following race can lead to a loss of i_disksize update from truncate
thus resulting in a wrong inode size if the inode size isn't updated
again before inode is reclaimed:
ext4_setattr() mpage_map_and_submit_extent()
EXT4_I(inode)->i_disksize = attr->ia_size;
... ...
disksize = ((loff_t)mpd->first_page) << PAGE_CACHE_SHIFT
/* False because i_size isn't
* updated yet */
if (disksize > i_size_read(inode))
/* True, because i_disksize is
* already truncated */
if (disksize > EXT4_I(inode)->i_disksize)
/* Overwrite i_disksize
* update from truncate */
ext4_update_i_disksize()
i_size_write(inode, attr->ia_size);
For other places updating i_disksize such race cannot happen because
i_mutex prevents these races. Writeback is the only place where we do
not hold i_mutex and we cannot grab it there because of lock ordering.
We fix the race by doing both i_disksize and i_size update in truncate
Atomically under i_data_sem and in mpage_map_and_submit_extent() we move
the check against i_size under i_data_sem as well.
So your proposed fix would introduce a regression by re-enabling the
bug which is fixed by commit 90e775b71ac4.
In any case, as Andreas has pointed out, this is a false positive; the
supposed deadlock involves an ea_inode in stack trace #0, whereas the
stack trace #1 involves a write to a data inode. Andreas has
suggested fixing this by annotating the lock appropriately. This case
is not going to happen in real production systems today, since
triggering it requires using the debugging mount option
debug_want_extra_isize.
So while it would be good to avoid the false positive lockdep warning,
fixing this is a lower priority bug --- it certainly isn't security
issue that syzbot developers like to point at when talking about the
"Linux security disaster". It isn't even a real production level bug!
Cheers,
- Ted
© 2016 - 2026 Red Hat, Inc.