Hi,
I've a following syzkaller report (no reproducer); the report is
against 5.15, but the same call-chain seems possible in current
upstream as well. So I suspect that maybe ext4_xattr_inode_create()
should take nested inode_lock (I_MUTEX_XATTR) instead. Does the
patch below make any sense?
======================================================
WARNING: possible circular locking dependency detected
5.15.168-syzkaller-23766-g3f37c55c6291 #0 Not tainted
------------------------------------------------------
syz-executor297/1452 is trying to acquire lock:
ffff888120b5e750 (&ea_inode->i_rwsem#8/1){+.+.}-{3:3}, at: inode_lock
ffff888120b5e750 (&ea_inode->i_rwsem#8/1){+.+.}-{3:3}, at: ext4_xattr_inode_create
ffff888120b5e750 (&ea_inode->i_rwsem#8/1){+.+.}-{3:3}, at: ext4_xattr_inode_lookup_create
ffff888120b5e750 (&ea_inode->i_rwsem#8/1){+.+.}-{3:3}, at: ext4_xattr_set_entry+0x2aeb/0x3200
but task is already holding lock:
ffff888120b58c68 (&ei->i_data_sem/3){++++}-{3:3}, at: ext4_setattr+0x12b5/0x1950
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+0x38/0x60
ext4_update_i_disksize
ext4_xattr_inode_write
ext4_xattr_inode_lookup_create
ext4_xattr_set_entry+0x2839/0x3200
ext4_xattr_ibody_set+0x113/0x320
ext4_xattr_set_handle+0xa31/0x1440
ext4_xattr_set+0x266/0x3d0
__vfs_setxattr+0x15e/0x1c0
__vfs_setxattr_noperm+0x128/0x5e0
vfs_setxattr+0x1c6/0x410
setxattr+0x1d6/0x270
path_setxattr+0x1cc/0x2b0
__do_sys_lsetxattr
__se_sys_lsetxattr
__x64_sys_lsetxattr+0xb4/0xd0
do_syscall_x64
do_syscall_64+0x69/0xc0
entry_SYSCALL_64_after_hwframe+0x66/0xd0
-> #0 (&ea_inode->i_rwsem#8/1){+.+.}-{3:3}:
check_prev_add
check_prevs_add
validate_chain
__lock_acquire+0x2c95/0x7850
lock_acquire+0x1d2/0x4e0
down_write+0x38/0x60
inode_lock
ext4_xattr_inode_create
ext4_xattr_inode_lookup_create
ext4_xattr_set_entry+0x2aeb/0x3200
ext4_xattr_block_set+0xdc1/0x2de0
ext4_xattr_move_to_block
ext4_xattr_make_inode_space
ext4_expand_extra_isize_ea+0xe58/0x19c0
__ext4_expand_extra_isize+0x2fd/0x400
ext4_try_to_expand_extra_isize
__ext4_mark_inode_dirty+0x58b/0x840
ext4_setattr+0x1341/0x1950
notify_change+0xafb/0xd80
do_truncate+0x218/0x2f0
handle_truncate
do_open
path_openat+0x27d3/0x2e10
do_filp_open+0x23a/0x360
do_sys_openat2+0x188/0x720
do_sys_open+0x1d1/0x220
do_syscall_x64
do_syscall_64+0x69/0xc0
entry_SYSCALL_64_after_hwframe+0x66/0xd0
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 ***
5 locks held by syz-executor297/1452:
#0: ffff88811231c460 (sb_writers#5){.+.+}-{0:0}, at: mnt_want_write+0x3b/0x80
#1: ffff888120b58de0 (&sb->s_type->i_mutex_key#8){++++}-{3:3}, at: inode_lock
#1: ffff888120b58de0 (&sb->s_type->i_mutex_key#8){++++}-{3:3}, at: do_truncate+0x204/0x2f0
#2: ffff888120b58f80 (mapping.invalidate_lock){++++}-{3:3}, at: filemap_invalidate_lock
#2: ffff888120b58f80 (mapping.invalidate_lock){++++}-{3:3}, at: ext4_setattr+0xd49/0x1950
#3: ffff888120b58c68 (&ei->i_data_sem/3){++++}-{3:3}, at: ext4_setattr+0x12b5/0x1950
#4: ffff888120b58ab8 (&ei->xattr_sem){++++}-{3:3}, at: ext4_write_trylock_xattr
#4: ffff888120b58ab8 (&ei->xattr_sem){++++}-{3:3}, at: ext4_try_to_expand_extra_isize
#4: ffff888120b58ab8 (&ei->xattr_sem){++++}-{3:3}, at: __ext4_mark_inode_dirty+0x4f7/0x840
stack backtrace:
Call Trace:
<TASK>
__dump_stack
dump_stack_lvl+0x1e3/0x2d0
check_noncircular+0x2f3/0x3a0
check_prev_add
check_prevs_add
validate_chain
__lock_acquire+0x2c95/0x7850
lock_acquire+0x1d2/0x4e0
down_write+0x38/0x60
inode_lock
ext4_xattr_inode_create
ext4_xattr_inode_lookup_create
ext4_xattr_set_entry+0x2aeb/0x3200
ext4_xattr_block_set+0xdc1/0x2de0
ext4_xattr_move_to_block
ext4_xattr_make_inode_space
ext4_expand_extra_isize_ea+0xe58/0x19c0
__ext4_expand_extra_isize+0x2fd/0x400
ext4_try_to_expand_extra_isize
__ext4_mark_inode_dirty+0x58b/0x840
ext4_setattr+0x1341/0x1950
notify_change+0xafb/0xd80
do_truncate+0x218/0x2f0
handle_truncate
do_open
path_openat+0x27d3/0x2e10
do_filp_open+0x23a/0x360
do_sys_openat2+0x188/0x720
do_sys_open+0x1d1/0x220
do_syscall_x64
do_syscall_64+0x69/0xc0
entry_SYSCALL_64_after_hwframe+0x66/0xd0
---
diff --git a/fs/ext4/xattr.c b/fs/ext4/xattr.c
index 7647e9f6e190..db3c68fbbadf 100644
--- a/fs/ext4/xattr.c
+++ b/fs/ext4/xattr.c
@@ -1511,7 +1511,7 @@ static struct inode *ext4_xattr_inode_create(handle_t *handle,
*/
dquot_free_inode(ea_inode);
dquot_drop(ea_inode);
- inode_lock(ea_inode);
+ inode_lock_nested(inode, I_MUTEX_XATTR);
ea_inode->i_flags |= S_NOQUOTA;
inode_unlock(ea_inode);
}
