Fuzzing reports a possible deadlock in jbd2_log_wait_commit.
The problem occurs in ext4_ind_migrate due to an incorrect order of unlocking of the journal and write semaphores - the order of unlocking must be the reverse of the order of locking.
Found by Linux Verification Center (linuxtesting.org) with syzkaller.
Signed-off-by: Artem Sadovnikov ancowi69@gmail.com --- fs/ext4/migrate.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/ext4/migrate.c b/fs/ext4/migrate.c index d98ac2af8199..a5e1492bbaaa 100644 --- a/fs/ext4/migrate.c +++ b/fs/ext4/migrate.c @@ -663,8 +663,8 @@ int ext4_ind_migrate(struct inode *inode) if (unlikely(ret2 && !ret)) ret = ret2; errout: - ext4_journal_stop(handle); up_write(&EXT4_I(inode)->i_data_sem); + ext4_journal_stop(handle); out_unlock: ext4_writepages_up_write(inode->i_sb, alloc_ctx); return ret;
Hi,
Thanks for your patch.
FYI: kernel test robot notices the stable kernel rule is not satisfied.
The check is based on https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html#opti...
Rule: add the tag "Cc: stable@vger.kernel.org" in the sign-off area to have the patch automatically included in the stable tree. Subject: [PATCH] ext4: fix i_data_sem unlock order in ext4_ind_migrate() Link: https://lore.kernel.org/stable/20240404095000.5872-1-mish.uxin2012%40yandex....
On Thu, Apr 04, 2024 at 12:50:00PM +0300, Mikhail Ukhin wrote:
Fuzzing reports a possible deadlock in jbd2_log_wait_commit.
The problem occurs in ext4_ind_migrate due to an incorrect order of unlocking of the journal and write semaphores - the order of unlocking must be the reverse of the order of locking.
Found by Linux Verification Center (linuxtesting.org) with syzkaller.
Signed-off-by: Artem Sadovnikov ancowi69@gmail.com
Does not match your From: line :(
fs/ext4/migrate.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/fs/ext4/migrate.c b/fs/ext4/migrate.c index d98ac2af8199..a5e1492bbaaa 100644 --- a/fs/ext4/migrate.c +++ b/fs/ext4/migrate.c @@ -663,8 +663,8 @@ int ext4_ind_migrate(struct inode *inode) if (unlikely(ret2 && !ret)) ret = ret2; errout:
- ext4_journal_stop(handle); up_write(&EXT4_I(inode)->i_data_sem);
- ext4_journal_stop(handle);
out_unlock: ext4_writepages_up_write(inode->i_sb, alloc_ctx); return ret; -- 2.25.1
<formletter>
This is not the correct way to submit patches for inclusion in the stable kernel tree. Please read: https://www.kernel.org/doc/html/latest/process/stable-kernel-rules.html for how to do this properly.
</formletter>
On Thu, Apr 04, 2024 at 12:50:00PM +0300, Mikhail Ukhin wrote:
Fuzzing reports a possible deadlock in jbd2_log_wait_commit.
The problem occurs in ext4_ind_migrate due to an incorrect order of unlocking of the journal and write semaphores - the order of unlocking must be the reverse of the order of locking.
Found by Linux Verification Center (linuxtesting.org) with syzkaller.
This doesn't make any sense. Lock order matters; the order in which you unlock shouldn't (and doesn't) make a difference. This is also something which lockdep doesn't complain about --- because it's not a problem.
So how was this "found by syzkaller"?
- Ted
On Friday, April 5, 2024 5:26:51 AM MSK Theodore Ts'o wrote:
This doesn't make any sense. Lock order matters; the order in which you unlock shouldn't (and doesn't) make a difference. This is also something which lockdep doesn't complain about --- because it's not a problem.
So how was this "found by syzkaller"?
- Ted
This issue only occurs when CONFIG_PROVE_LOCKING is set, in which case jbd2_might_wait_for_commit macro will lock jbd2_handle in jbd2_journal_stop function, which, while i_data_sem is locked, will trigger lockdep, because jbd2_journal_start function might also lock on same jbd2_handle at the same time
Without CONFIG_PROVE_LOCKING this issue is not possible because jbd2_journal_stop never calls jbd2_might_wait_for_commit
Since syzkaller report was local and wasn't inserted in initial patch message, I will put it in this message It wasn't been able to create a reproducer for that problem, so there's only a crash report
====================================================== WARNING: possible circular locking dependency detected 5.10.208-syzkaller #0 Not tainted ------------------------------------------------------ syz-fuzzer/1080 is trying to acquire lock: ffff88810b09e8e0 (jbd2_handle){++++}-{0:0}, at: jbd2_log_wait_commit+0x142/0x430 fs/jbd2/journal.c:693
but task is already holding lock: ffff88805eec5c78 (&ei->i_data_sem){++++}-{3:3}, at: ext4_ind_migrate+0x2fe/0x840 fs/ext4/migrate.c:633
which lock already depends on the new lock.
the existing dependency chain (in reverse order) is:
-> #1 (&ei->i_data_sem){++++}-{3:3}: down_read+0x96/0x420 kernel/locking/rwsem.c:1504 ext4_da_map_blocks fs/ext4/inode.c:1776 [inline] ext4_da_get_block_prep+0x5b4/0x11a0 fs/ext4/inode.c:1857 ext4_block_write_begin+0x479/0x1230 fs/ext4/inode.c:1076 ext4_da_write_begin+0x3ca/0x1060 fs/ext4/inode.c:3063 generic_perform_write+0x210/0x500 mm/filemap.c:3336 ext4_buffered_write_iter+0x232/0x4a0 fs/ext4/file.c:269 ext4_file_write_iter+0x429/0x1420 fs/ext4/file.c:683 call_write_iter include/linux/fs.h:1962 [inline] new_sync_write+0x427/0x660 fs/read_write.c:518 vfs_write+0x774/0xab0 fs/read_write.c:605 ksys_write+0x12d/0x260 fs/read_write.c:658 do_syscall_64+0x33/0x40 arch/x86/entry/common.c:46 entry_SYSCALL_64_after_hwframe+0x62/0xc7
-> #0 (jbd2_handle){++++}-{0:0}: check_prev_add kernel/locking/lockdep.c:2988 [inline] check_prevs_add kernel/locking/lockdep.c:3113 [inline] validate_chain kernel/locking/lockdep.c:3729 [inline] __lock_acquire+0x29c4/0x53c0 kernel/locking/lockdep.c:4955 lock_acquire kernel/locking/lockdep.c:5566 [inline] lock_acquire+0x19f/0x4a0 kernel/locking/lockdep.c:5531 jbd2_log_wait_commit+0x177/0x430 fs/jbd2/journal.c:696 jbd2_journal_stop+0x5b0/0xde0 fs/jbd2/transaction.c:1933 __ext4_journal_stop+0xde/0x1f0 fs/ext4/ext4_jbd2.c:127 ext4_ind_migrate+0x402/0x840 fs/ext4/migrate.c:666 ext4_ioctl_setflags+0xaef/0xc70 fs/ext4/ioctl.c:459 __ext4_ioctl+0x3742/0x4e20 fs/ext4/ioctl.c:870 vfs_ioctl fs/ioctl.c:48 [inline] __do_sys_ioctl fs/ioctl.c:753 [inline] __se_sys_ioctl fs/ioctl.c:739 [inline] __x64_sys_ioctl+0x19a/0x210 fs/ioctl.c:739 do_syscall_64+0x33/0x40 arch/x86/entry/common.c:46 entry_SYSCALL_64_after_hwframe+0x62/0xc7
other info that might help us debug this:
Possible unsafe locking scenario:
CPU0 CPU1 ---- ---- lock(&ei->i_data_sem); lock(jbd2_handle); lock(&ei->i_data_sem); lock(jbd2_handle);
*** DEADLOCK ***
4 locks held by syz-fuzzer/1080: #0: ffff88810bc42460 (sb_writers#4){.+.+}-{0:0}, at: __ext4_ioctl+0x76a/0x4e20 fs/ext4/ioctl.c:861 #1: ffff88805eec5e88 (&type->i_mutex_dir_key#3){++++}-{3:3}, at: inode_lock include/linux/fs.h:774 [inline] #1: ffff88805eec5e88 (&type->i_mutex_dir_key#3){++++}-{3:3}, at: __ext4_ioctl+0x799/0x4e20 fs/ext4/ioctl.c:865 #2: ffff88810bc44ac0 (&sbi->s_writepages_rwsem){++++}-{0:0}, at: ext4_ind_migrate+0x237/0x840 fs/ext4/migrate.c:625 #3: ffff88805eec5c78 (&ei->i_data_sem){++++}-{3:3}, at: ext4_ind_migrate+0x2fe/ 0x840 fs/ext4/migrate.c:633
stack backtrace: CPU: 0 PID: 1080 Comm: syz-fuzzer Not tainted 5.10.208-syzkaller #0 Hardware name: QEMU Standard PC (i440FX + PIIX, 1996), BIOS 1.12.0-1 04/01/2014 Call Trace: __dump_stack lib/dump_stack.c:77 [inline] dump_stack+0x107/0x167 lib/dump_stack.c:118 check_noncircular+0x26c/0x320 kernel/locking/lockdep.c:2123 check_prev_add kernel/locking/lockdep.c:2988 [inline] check_prevs_add kernel/locking/lockdep.c:3113 [inline] validate_chain kernel/locking/lockdep.c:3729 [inline] __lock_acquire+0x29c4/0x53c0 kernel/locking/lockdep.c:4955 lock_acquire kernel/locking/lockdep.c:5566 [inline] lock_acquire+0x19f/0x4a0 kernel/locking/lockdep.c:5531 jbd2_log_wait_commit+0x177/0x430 fs/jbd2/journal.c:696 jbd2_journal_stop+0x5b0/0xde0 fs/jbd2/transaction.c:1933 __ext4_journal_stop+0xde/0x1f0 fs/ext4/ext4_jbd2.c:127 ext4_ind_migrate+0x402/0x840 fs/ext4/migrate.c:666 ext4_ioctl_setflags+0xaef/0xc70 fs/ext4/ioctl.c:459 __ext4_ioctl+0x3742/0x4e20 fs/ext4/ioctl.c:870 vfs_ioctl fs/ioctl.c:48 [inline] __do_sys_ioctl fs/ioctl.c:753 [inline] __se_sys_ioctl fs/ioctl.c:739 [inline] __x64_sys_ioctl+0x19a/0x210 fs/ioctl.c:739 do_syscall_64+0x33/0x40 arch/x86/entry/common.c:46 entry_SYSCALL_64_after_hwframe+0x62/0xc7 RIP: 0033:0x49e0bb Code: e8 0a 4b fc ff eb 88 cc cc cc cc cc cc cc cc e8 1b 8f fc ff 48 8b 7c 24 10 48 8b 74 24 18 48 8b 54 24 20 48 8b 44 24 08 0f 05 <48> 3d 01 f0 ff ff 76 20 48 c7 44 24 28 ff ff ff ff 48 c7 44 24 30 RSP: 002b:000000c002b65ae8 EFLAGS: 00000212 ORIG_RAX: 0000000000000010 RAX: ffffffffffffffda RBX: 000000c000025000 RCX: 000000000049e0bb RDX: 000000c002b65b64 RSI: 0000000040086602 RDI: 000000000000001b RBP: 000000c002b65b28 R08: 0000000000000001 R09: 0000000000000000 R10: 0000000000000000 R11: 0000000000000212 R12: 000000c02b38c840 R13: 0000000000000001 R14: 000000c018a321a0 R15: ffffffffffffffff
linux-stable-mirror@lists.linaro.org
-
Artem Sadovnikov -
Greg KH -
kernel test robot -
Mikhail Ukhin -
Theodore Ts'o