All of lore.kernel.org
 help / color / mirror / Atom feed
From: Edward Adam Davis <eadavis@qq.com>
To: syzbot+ee72b9a7aad1e5a77c5c@syzkaller.appspotmail.com
Cc: adilger.kernel@dilger.ca, linux-ext4@vger.kernel.org,
	linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
	syzkaller-bugs@googlegroups.com, tytso@mit.edu
Subject: [PATCH] ext4: fix deadlock in ext4_xattr_inode_iget
Date: Thu,  4 Apr 2024 09:54:02 +0800	[thread overview]
Message-ID: <tencent_9E9EB81B474B0E1B23256EBA05BB79332408@qq.com> (raw)
In-Reply-To: <000000000000163e1406152c6877@google.com>

[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.0


  parent reply	other threads:[~2024-04-04  2:00 UTC|newest]

Thread overview: 5+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-04-03  7:45 [syzbot] [ext4?] possible deadlock in ext4_xattr_inode_iget (3) syzbot
2024-04-03 13:44 ` Hillf Danton
2024-04-04  0:48   ` syzbot
2024-04-04  1:54 ` Edward Adam Davis [this message]
2024-04-08 21:38 ` Andreas Dilger

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=tencent_9E9EB81B474B0E1B23256EBA05BB79332408@qq.com \
    --to=eadavis@qq.com \
    --cc=adilger.kernel@dilger.ca \
    --cc=linux-ext4@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=syzbot+ee72b9a7aad1e5a77c5c@syzkaller.appspotmail.com \
    --cc=syzkaller-bugs@googlegroups.com \
    --cc=tytso@mit.edu \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.