Hello,
syzbot found the following issue on:
HEAD commit: bee0e7762ad2 Merge tag 'for-linus-iommufd' of git://git.ke..
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=1500ff54e80000
kernel config: https://syzkaller.appspot.com/x/.config?x=b45dfd882e46ec91
dashboard link: https://syzkaller.appspot.com/bug?extid=41a88b825a315aac2254
compiler: Debian clang version 15.0.6, GNU ld (GNU Binutils for Debian) 2.40
Unfortunately, I don't have any reproducer for this issue yet.
Downloadable assets:
disk image: https://storage.googleapis.com/syzbot-assets/af357ba4767f/disk-bee0e776.raw.xz
vmlinux: https://storage.googleapis.com/syzbot-assets/ae4d50206171/vmlinux-bee0e776.xz
kernel image: https://storage.googleapis.com/syzbot-assets/e12203376a9f/bzImage-bee0e776.xz
IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: [email protected]
loop5: detected capacity change from 0 to 64
======================================================
WARNING: possible circular locking dependency detected
6.7.0-rc4-syzkaller-00009-gbee0e7762ad2 #0 Not tainted
------------------------------------------------------
syz-executor.5/10381 is trying to acquire lock:
ffff888080716f78 (&HFS_I(tree->inode)->extents_lock){+.+.}-{3:3}, at: hfs_extend_file+0xff/0x1440 fs/hfs/extent.c:397
but task is already holding lock:
ffff8880396060b0 (&tree->tree_lock#2/1){+.+.}-{3:3}, at: hfs_find_init+0x16e/0x1f0
which lock already depends on the new lock.
the existing dependency chain (in reverse order) is:
-> #1 (&tree->tree_lock#2/1){+.+.}-{3:3}:
lock_acquire+0x1e3/0x530 kernel/locking/lockdep.c:5754
__mutex_lock_common kernel/locking/mutex.c:603 [inline]
__mutex_lock+0x136/0xd60 kernel/locking/mutex.c:747
hfs_find_init+0x16e/0x1f0
hfs_ext_read_extent fs/hfs/extent.c:200 [inline]
hfs_extend_file+0x31b/0x1440 fs/hfs/extent.c:401
hfs_bmap_reserve+0xd9/0x3f0 fs/hfs/btree.c:234
hfs_cat_create+0x1e0/0x970 fs/hfs/catalog.c:104
hfs_create+0x66/0xd0 fs/hfs/dir.c:202
lookup_open fs/namei.c:3477 [inline]
open_last_lookups fs/namei.c:3546 [inline]
path_openat+0x13fa/0x3290 fs/namei.c:3776
do_filp_open+0x234/0x490 fs/namei.c:3809
do_sys_openat2+0x13e/0x1d0 fs/open.c:1440
do_sys_open fs/open.c:1455 [inline]
__do_sys_openat fs/open.c:1471 [inline]
__se_sys_openat fs/open.c:1466 [inline]
__x64_sys_openat+0x247/0x290 fs/open.c:1466
do_syscall_x64 arch/x86/entry/common.c:51 [inline]
do_syscall_64+0x45/0x110 arch/x86/entry/common.c:82
entry_SYSCALL_64_after_hwframe+0x63/0x6b
-> #0 (&HFS_I(tree->inode)->extents_lock){+.+.}-{3:3}:
check_prev_add kernel/locking/lockdep.c:3134 [inline]
check_prevs_add kernel/locking/lockdep.c:3253 [inline]
validate_chain+0x1909/0x5ab0 kernel/locking/lockdep.c:3869
__lock_acquire+0x1345/0x1fd0 kernel/locking/lockdep.c:5137
lock_acquire+0x1e3/0x530 kernel/locking/lockdep.c:5754
__mutex_lock_common kernel/locking/mutex.c:603 [inline]
__mutex_lock+0x136/0xd60 kernel/locking/mutex.c:747
hfs_extend_file+0xff/0x1440 fs/hfs/extent.c:397
hfs_bmap_reserve+0xd9/0x3f0 fs/hfs/btree.c:234
__hfs_ext_write_extent+0x22e/0x4f0 fs/hfs/extent.c:121
__hfs_ext_cache_extent+0x6a/0x990 fs/hfs/extent.c:174
hfs_ext_read_extent fs/hfs/extent.c:202 [inline]
hfs_extend_file+0x344/0x1440 fs/hfs/extent.c:401
hfs_get_block+0x3e4/0xb60 fs/hfs/extent.c:353
__block_write_begin_int+0x54d/0x1ad0 fs/buffer.c:2119
__block_write_begin fs/buffer.c:2168 [inline]
block_write_begin+0x9b/0x1e0 fs/buffer.c:2227
cont_write_begin+0x643/0x880 fs/buffer.c:2582
hfs_write_begin+0x8a/0xd0 fs/hfs/inode.c:58
generic_perform_write+0x31b/0x630 mm/filemap.c:3918
generic_file_write_iter+0xaf/0x310 mm/filemap.c:4039
call_write_iter include/linux/fs.h:2020 [inline]
new_sync_write fs/read_write.c:491 [inline]
vfs_write+0x792/0xb20 fs/read_write.c:584
ksys_write+0x1a0/0x2c0 fs/read_write.c:637
do_syscall_x64 arch/x86/entry/common.c:51 [inline]
do_syscall_64+0x45/0x110 arch/x86/entry/common.c:82
entry_SYSCALL_64_after_hwframe+0x63/0x6b
other info that might help us debug this:
Possible unsafe locking scenario:
CPU0 CPU1
---- ----
lock(&tree->tree_lock#2/1);
lock(&HFS_I(tree->inode)->extents_lock);
lock(&tree->tree_lock#2/1);
lock(&HFS_I(tree->inode)->extents_lock);
*** DEADLOCK ***
5 locks held by syz-executor.5/10381:
#0: ffff888018664fc8 (&f->f_pos_lock){+.+.}-{3:3}, at: __fdget_pos+0x2b0/0x340 fs/file.c:1177
#1: ffff88807234c418 (sb_writers#20){.+.+}-{0:0}, at: vfs_write+0x223/0xb20 fs/read_write.c:580
#2: ffff888080715da8 (&sb->s_type->i_mutex_key#28){+.+.}-{3:3}, at: inode_lock include/linux/fs.h:802 [inline]
#2: ffff888080715da8 (&sb->s_type->i_mutex_key#28){+.+.}-{3:3}, at: generic_file_write_iter+0x83/0x310 mm/filemap.c:4036
#3: ffff888080715bf8 (&HFS_I(inode)->extents_lock#2){+.+.}-{3:3}, at: hfs_extend_file+0xff/0x1440 fs/hfs/extent.c:397
#4: ffff8880396060b0 (&tree->tree_lock#2/1){+.+.}-{3:3}, at: hfs_find_init+0x16e/0x1f0
stack backtrace:
CPU: 1 PID: 10381 Comm: syz-executor.5 Not tainted 6.7.0-rc4-syzkaller-00009-gbee0e7762ad2 #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 11/10/2023
Call Trace:
<TASK>
__dump_stack lib/dump_stack.c:88 [inline]
dump_stack_lvl+0x1e7/0x2d0 lib/dump_stack.c:106
check_noncircular+0x366/0x490 kernel/locking/lockdep.c:2187
check_prev_add kernel/locking/lockdep.c:3134 [inline]
check_prevs_add kernel/locking/lockdep.c:3253 [inline]
validate_chain+0x1909/0x5ab0 kernel/locking/lockdep.c:3869
__lock_acquire+0x1345/0x1fd0 kernel/locking/lockdep.c:5137
lock_acquire+0x1e3/0x530 kernel/locking/lockdep.c:5754
__mutex_lock_common kernel/locking/mutex.c:603 [inline]
__mutex_lock+0x136/0xd60 kernel/locking/mutex.c:747
hfs_extend_file+0xff/0x1440 fs/hfs/extent.c:397
hfs_bmap_reserve+0xd9/0x3f0 fs/hfs/btree.c:234
__hfs_ext_write_extent+0x22e/0x4f0 fs/hfs/extent.c:121
__hfs_ext_cache_extent+0x6a/0x990 fs/hfs/extent.c:174
hfs_ext_read_extent fs/hfs/extent.c:202 [inline]
hfs_extend_file+0x344/0x1440 fs/hfs/extent.c:401
hfs_get_block+0x3e4/0xb60 fs/hfs/extent.c:353
__block_write_begin_int+0x54d/0x1ad0 fs/buffer.c:2119
__block_write_begin fs/buffer.c:2168 [inline]
block_write_begin+0x9b/0x1e0 fs/buffer.c:2227
cont_write_begin+0x643/0x880 fs/buffer.c:2582
hfs_write_begin+0x8a/0xd0 fs/hfs/inode.c:58
generic_perform_write+0x31b/0x630 mm/filemap.c:3918
generic_file_write_iter+0xaf/0x310 mm/filemap.c:4039
call_write_iter include/linux/fs.h:2020 [inline]
new_sync_write fs/read_write.c:491 [inline]
vfs_write+0x792/0xb20 fs/read_write.c:584
ksys_write+0x1a0/0x2c0 fs/read_write.c:637
do_syscall_x64 arch/x86/entry/common.c:51 [inline]
do_syscall_64+0x45/0x110 arch/x86/entry/common.c:82
entry_SYSCALL_64_after_hwframe+0x63/0x6b
RIP: 0033:0x7f8f6167cae9
Code: 28 00 00 00 75 05 48 83 c4 28 c3 e8 e1 20 00 00 90 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 b0 ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007f8f6230d0c8 EFLAGS: 00000246 ORIG_RAX: 0000000000000001
RAX: ffffffffffffffda RBX: 00007f8f6179bf80 RCX: 00007f8f6167cae9
RDX: 00000000000ffe00 RSI: 0000000020004200 RDI: 0000000000000004
RBP: 00007f8f616c847a R08: 0000000000000000 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 0000000000000000
R13: 000000000000000b R14: 00007f8f6179bf80 R15: 00007ffcca30fd58
</TASK>
---
This report is generated by a bot. It may contain errors.
See https://goo.gl/tpsmEJ for more information about syzbot.
syzbot engineers can be reached at [email protected].
syzbot will keep track of this issue. See:
https://goo.gl/tpsmEJ#status for how to communicate with syzbot.
If the report is already addressed, let syzbot know by replying with:
#syz fix: exact-commit-title
If you want to overwrite report's subsystems, reply with:
#syz set subsystems: new-subsystem
(See the list of subsystem names on the web dashboard)
If the report is a duplicate of another one, reply with:
#syz dup: exact-subject-of-another-report
If you want to undo deduplication, reply with:
#syz undup
syzbot has found a reproducer for the following issue on:
HEAD commit: 610a9b8f49fb Linux 6.7-rc8
git tree: upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=16d7c48de80000
kernel config: https://syzkaller.appspot.com/x/.config?x=247b5a935d307ee5
dashboard link: https://syzkaller.appspot.com/bug?extid=41a88b825a315aac2254
compiler: gcc (Debian 12.2.0-14) 12.2.0, GNU ld (GNU Binutils for Debian) 2.40
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=1552fe19e80000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=1419bcade80000
Downloadable assets:
disk image: https://storage.googleapis.com/syzbot-assets/e174ec82158f/disk-610a9b8f.raw.xz
vmlinux: https://storage.googleapis.com/syzbot-assets/4bed5e1c1c26/vmlinux-610a9b8f.xz
kernel image: https://storage.googleapis.com/syzbot-assets/4fd13b65ecb5/bzImage-610a9b8f.xz
mounted in repro #1: https://storage.googleapis.com/syzbot-assets/19a994dad52e/mount_0.gz
mounted in repro #2: https://storage.googleapis.com/syzbot-assets/8c8468d1fd79/mount_1.gz
IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: [email protected]
loop0: detected capacity change from 0 to 64
============================================
WARNING: possible recursive locking detected
6.7.0-rc8-syzkaller #0 Not tainted
--------------------------------------------
syz-executor279/5059 is trying to acquire lock:
ffff888079c100f8 (&HFS_I(tree->inode)->extents_lock){+.+.}-{3:3}, at: hfs_extend_file+0xa2/0xb10 fs/hfs/extent.c:397
but task is already holding lock:
ffff888079c10778 (&HFS_I(tree->inode)->extents_lock){+.+.}-{3:3}, at: hfs_extend_file+0xa2/0xb10 fs/hfs/extent.c:397
other info that might help us debug this:
Possible unsafe locking scenario:
CPU0
----
lock(&HFS_I(tree->inode)->extents_lock);
lock(&HFS_I(tree->inode)->extents_lock);
*** DEADLOCK ***
May be due to missing lock nesting notation
5 locks held by syz-executor279/5059:
#0: ffff88807a160418 (sb_writers#9){.+.+}-{0:0}, at: open_last_lookups fs/namei.c:3535 [inline]
#0: ffff88807a160418 (sb_writers#9){.+.+}-{0:0}, at: path_openat+0x19f6/0x2c50 fs/namei.c:3776
#1: ffff888079c10fa8 (&type->i_mutex_dir_key#6){+.+.}-{3:3}, at: inode_lock include/linux/fs.h:802 [inline]
#1: ffff888079c10fa8 (&type->i_mutex_dir_key#6){+.+.}-{3:3}, at: open_last_lookups fs/namei.c:3543 [inline]
#1: ffff888079c10fa8 (&type->i_mutex_dir_key#6){+.+.}-{3:3}, at: path_openat+0x8bd/0x2c50 fs/namei.c:3776
#2: ffff88807a1640b0 (&tree->tree_lock){+.+.}-{3:3}, at: hfs_find_init+0x1b6/0x220 fs/hfs/bfind.c:30
#3: ffff888079c10778 (&HFS_I(tree->inode)->extents_lock){+.+.}-{3:3}, at: hfs_extend_file+0xa2/0xb10 fs/hfs/extent.c:397
#4: ffff88807a1620b0 (&tree->tree_lock/1){+.+.}-{3:3}, at: hfs_find_init+0x17f/0x220 fs/hfs/bfind.c:33
stack backtrace:
CPU: 0 PID: 5059 Comm: syz-executor279 Not tainted 6.7.0-rc8-syzkaller #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 11/17/2023
Call Trace:
<TASK>
__dump_stack lib/dump_stack.c:88 [inline]
dump_stack_lvl+0xd9/0x1b0 lib/dump_stack.c:106
check_deadlock kernel/locking/lockdep.c:3062 [inline]
validate_chain kernel/locking/lockdep.c:3856 [inline]
__lock_acquire+0x20f8/0x3b20 kernel/locking/lockdep.c:5137
lock_acquire kernel/locking/lockdep.c:5754 [inline]
lock_acquire+0x1ae/0x520 kernel/locking/lockdep.c:5719
__mutex_lock_common kernel/locking/mutex.c:603 [inline]
__mutex_lock+0x175/0x9d0 kernel/locking/mutex.c:747
hfs_extend_file+0xa2/0xb10 fs/hfs/extent.c:397
hfs_bmap_reserve+0x29c/0x370 fs/hfs/btree.c:234
__hfs_ext_write_extent+0x3cb/0x520 fs/hfs/extent.c:121
__hfs_ext_cache_extent fs/hfs/extent.c:174 [inline]
hfs_ext_read_extent+0x805/0x9d0 fs/hfs/extent.c:202
hfs_extend_file+0x4e0/0xb10 fs/hfs/extent.c:401
hfs_bmap_reserve+0x29c/0x370 fs/hfs/btree.c:234
hfs_cat_create+0x227/0x810 fs/hfs/catalog.c:104
hfs_create+0x67/0xe0 fs/hfs/dir.c:202
lookup_open.isra.0+0x1095/0x13b0 fs/namei.c:3477
open_last_lookups fs/namei.c:3546 [inline]
path_openat+0x922/0x2c50 fs/namei.c:3776
do_filp_open+0x1de/0x430 fs/namei.c:3809
do_sys_openat2+0x176/0x1e0 fs/open.c:1437
do_sys_open fs/open.c:1452 [inline]
__do_sys_openat fs/open.c:1468 [inline]
__se_sys_openat fs/open.c:1463 [inline]
__x64_sys_openat+0x175/0x210 fs/open.c:1463
do_syscall_x64 arch/x86/entry/common.c:52 [inline]
do_syscall_64+0x40/0x110 arch/x86/entry/common.c:83
entry_SYSCALL_64_after_hwframe+0x63/0x6b
RIP: 0033:0x7f776291b759
Code: 28 00 00 00 75 05 48 83 c4 28 c3 e8 b1 18 00 00 90 48 89 f8 48 89 f7 48 89 d6 48 89 ca 4d 89 c2 4d 89 c8 4c 8b 4c 24 08 0f 05 <48> 3d 01 f0 ff ff 73 01 c3 48 c7 c1 b0 ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007f77628d7168 EFLAGS: 00000246 ORIG_RAX: 0000000000000101
RAX: ffffffffffffffda RBX: 00007f77629a46c8 RCX: 00007f776291b759
RDX: 000000000000275a RSI: 0000000020000000 RDI: 00000000ffffff9c
RBP: 00007f77629a46c0 R08: 00007f77629a46c0 R09: 0000000000000000
R10: 0000000000000000 R11: 0000000000000246 R12: 00007f77629a46cc
R13: 0000000000000006 R14: 00007ffd59ba19f0 R15: 00007ffd59ba1ad8
</TASK>
hfs: request for non-existent node 16777216 in B*Tree
hfs: request for non-existent node 16777216 in B*Tree
hfs: inconsistency in B*Tree (5,0,1,0,1)
---
If you want syzbot to run the reproducer, reply with:
#syz test: git://repo/address.git branch-or-commit-hash
If you attach or paste a git patch, syzbot will apply it before testing.
please test possible deadlock in hfs_extend_file
#syz test https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git 610a9b8f49fb
diff --git a/fs/hfs/extent.c b/fs/hfs/extent.c
index 6d1878b99b30..1b02c7b6a10c 100644
--- a/fs/hfs/extent.c
+++ b/fs/hfs/extent.c
@@ -197,6 +197,10 @@ static int hfs_ext_read_extent(struct inode *inode, u16 block)
block < HFS_I(inode)->cached_start + HFS_I(inode)->cached_blocks)
return 0;
+ if (HFS_I(inode)->flags & HFS_FLG_EXT_DIRTY &&
+ HFS_I(inode)->flags & HFS_FLG_EXT_NEW)
+ return -ENOENT;
+
res = hfs_find_init(HFS_SB(inode->i_sb)->ext_tree, &fd);
if (!res) {
res = __hfs_ext_cache_extent(&fd, inode, block);
Hello,
syzbot has tested the proposed patch and the reproducer did not trigger any issue:
Reported-and-tested-by: [email protected]
Tested on:
commit: 610a9b8f Linux 6.7-rc8
git tree: https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
console output: https://syzkaller.appspot.com/x/log.txt?x=14649d81e80000
kernel config: https://syzkaller.appspot.com/x/.config?x=247b5a935d307ee5
dashboard link: https://syzkaller.appspot.com/bug?extid=41a88b825a315aac2254
compiler: gcc (Debian 12.2.0-14) 12.2.0, GNU ld (GNU Binutils for Debian) 2.40
patch: https://syzkaller.appspot.com/x/patch.diff?x=1031326ee80000
Note: testing is done by a robot and is best-effort only.
[syz report]
syz-executor279/5059 is trying to acquire lock:
ffff888079c100f8 (&HFS_I(tree->inode)->extents_lock){+.+.}-{3:3}, at: hfs_extend_file+0xa2/0xb10 fs/hfs/extent.c:397
but task is already holding lock:
ffff888079c10778 (&HFS_I(tree->inode)->extents_lock){+.+.}-{3:3}, at: hfs_extend_file+0xa2/0xb10 fs/hfs/extent.c:397
other info that might help us debug this:
Possible unsafe locking scenario:
CPU0
----
lock(&HFS_I(tree->inode)->extents_lock);
lock(&HFS_I(tree->inode)->extents_lock);
*** DEADLOCK ***
[Analysis]
hfs_extend_file()->
hfs_ext_read_extent()->
__hfs_ext_cache_extent()->
__hfs_ext_write_extent()->
hfs_bmap_reserve()->
hfs_extend_file()->
When an inode has both the HFS_FLG_EXT_DIRTY and HFS_FLG_EXT_NEW flags, it will
enter the above loop and trigger a deadlock.
[Fix]
In hfs_ext_read_extent(), check if the above two flags exist simultaneously,
and exit the subsequent process when the conditions are met.
Reported-and-tested-by: [email protected]
Signed-off-by: Edward Adam Davis <[email protected]>
---
fs/hfs/extent.c | 4 ++++
1 file changed, 4 insertions(+)
diff --git a/fs/hfs/extent.c b/fs/hfs/extent.c
index 6d1878b99b30..1b02c7b6a10c 100644
--- a/fs/hfs/extent.c
+++ b/fs/hfs/extent.c
@@ -197,6 +197,10 @@ static int hfs_ext_read_extent(struct inode *inode, u16 block)
block < HFS_I(inode)->cached_start + HFS_I(inode)->cached_blocks)
return 0;
+ if (HFS_I(inode)->flags & HFS_FLG_EXT_DIRTY &&
+ HFS_I(inode)->flags & HFS_FLG_EXT_NEW)
+ return -ENOENT;
+
res = hfs_find_init(HFS_SB(inode->i_sb)->ext_tree, &fd);
if (!res) {
res = __hfs_ext_cache_extent(&fd, inode, block);
--
2.43.0
syzbot has bisected this issue to:
commit 54640c7502e5ed41fbf4eedd499e85f9acc9698f
Author: Ernesto A. Fernández <[email protected]>
Date: Tue Oct 30 22:06:17 2018 +0000
hfs: prevent btree data loss on ENOSPC
bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=1121e49ae80000
start commit: 610a9b8f49fb Linux 6.7-rc8
git tree: upstream
final oops: https://syzkaller.appspot.com/x/report.txt?x=1321e49ae80000
console output: https://syzkaller.appspot.com/x/log.txt?x=1521e49ae80000
kernel config: https://syzkaller.appspot.com/x/.config?x=247b5a935d307ee5
dashboard link: https://syzkaller.appspot.com/bug?extid=41a88b825a315aac2254
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=1552fe19e80000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=1419bcade80000
Reported-by: [email protected]
Fixes: 54640c7502e5 ("hfs: prevent btree data loss on ENOSPC")
For information about bisection process see: https://goo.gl/tpsmEJ#bisection
On Tue, Jan 02, 2024 at 08:36:51PM +0800, Edward Adam Davis wrote:
> [syz report]
> syz-executor279/5059 is trying to acquire lock:
> ffff888079c100f8 (&HFS_I(tree->inode)->extents_lock){+.+.}-{3:3}, at: hfs_extend_file+0xa2/0xb10 fs/hfs/extent.c:397
>
> but task is already holding lock:
> ffff888079c10778 (&HFS_I(tree->inode)->extents_lock){+.+.}-{3:3}, at: hfs_extend_file+0xa2/0xb10 fs/hfs/extent.c:397
>
> other info that might help us debug this:
> Possible unsafe locking scenario:
>
> CPU0
> ----
> lock(&HFS_I(tree->inode)->extents_lock);
> lock(&HFS_I(tree->inode)->extents_lock);
>
> *** DEADLOCK ***
> [Analysis]
> hfs_extend_file()->
> hfs_ext_read_extent()->
> __hfs_ext_cache_extent()->
> __hfs_ext_write_extent()->
> hfs_bmap_reserve()->
> hfs_extend_file()->
>
> When an inode has both the HFS_FLG_EXT_DIRTY and HFS_FLG_EXT_NEW flags, it will
> enter the above loop and trigger a deadlock.
>
> [Fix]
> In hfs_ext_read_extent(), check if the above two flags exist simultaneously,
> and exit the subsequent process when the conditions are met.
Why is this the correct fix? Seems to me that returning -ENOENT here is
going to lead to an error being reported to the user when the user has
done nothing wrong?
> Reported-and-tested-by: [email protected]
> Signed-off-by: Edward Adam Davis <[email protected]>
> ---
> fs/hfs/extent.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/fs/hfs/extent.c b/fs/hfs/extent.c
> index 6d1878b99b30..1b02c7b6a10c 100644
> --- a/fs/hfs/extent.c
> +++ b/fs/hfs/extent.c
> @@ -197,6 +197,10 @@ static int hfs_ext_read_extent(struct inode *inode, u16 block)
> block < HFS_I(inode)->cached_start + HFS_I(inode)->cached_blocks)
> return 0;
>
> + if (HFS_I(inode)->flags & HFS_FLG_EXT_DIRTY &&
> + HFS_I(inode)->flags & HFS_FLG_EXT_NEW)
> + return -ENOENT;
> +
> res = hfs_find_init(HFS_SB(inode->i_sb)->ext_tree, &fd);
> if (!res) {
> res = __hfs_ext_cache_extent(&fd, inode, block);
> --
> 2.43.0
>
>
> On Jan 2, 2024, at 3:36 PM, Edward Adam Davis <[email protected]> wrote:
>
> [syz report]
> syz-executor279/5059 is trying to acquire lock:
> ffff888079c100f8 (&HFS_I(tree->inode)->extents_lock){+.+.}-{3:3}, at: hfs_extend_file+0xa2/0xb10 fs/hfs/extent.c:397
>
> but task is already holding lock:
> ffff888079c10778 (&HFS_I(tree->inode)->extents_lock){+.+.}-{3:3}, at: hfs_extend_file+0xa2/0xb10 fs/hfs/extent.c:397
>
> other info that might help us debug this:
> Possible unsafe locking scenario:
>
> CPU0
> ----
> lock(&HFS_I(tree->inode)->extents_lock);
> lock(&HFS_I(tree->inode)->extents_lock);
>
> *** DEADLOCK ***
> [Analysis]
> hfs_extend_file()->
> hfs_ext_read_extent()->
> __hfs_ext_cache_extent()->
> __hfs_ext_write_extent()->
> hfs_bmap_reserve()->
> hfs_extend_file()->
>
> When an inode has both the HFS_FLG_EXT_DIRTY and HFS_FLG_EXT_NEW flags, it will
> enter the above loop and trigger a deadlock.
>
> [Fix]
> In hfs_ext_read_extent(), check if the above two flags exist simultaneously,
> and exit the subsequent process when the conditions are met.
>
> Reported-and-tested-by: [email protected]
> Signed-off-by: Edward Adam Davis <[email protected]>
> ---
> fs/hfs/extent.c | 4 ++++
> 1 file changed, 4 insertions(+)
>
> diff --git a/fs/hfs/extent.c b/fs/hfs/extent.c
> index 6d1878b99b30..1b02c7b6a10c 100644
> --- a/fs/hfs/extent.c
> +++ b/fs/hfs/extent.c
> @@ -197,6 +197,10 @@ static int hfs_ext_read_extent(struct inode *inode, u16 block)
> block < HFS_I(inode)->cached_start + HFS_I(inode)->cached_blocks)
> return 0;
>
> + if (HFS_I(inode)->flags & HFS_FLG_EXT_DIRTY &&
> + HFS_I(inode)->flags & HFS_FLG_EXT_NEW)
> + return -ENOENT;
> +
I don’t think that fix can be so simple. It looks like the code requires significant
refactoring. Because, currently, it looks like bad recursion: hfs_extend_file() finally
calls hfs_extend_file(). And it smells really badly. Also, from the logical point of view,
hfs_ext_read_extent() method calls __hfs_ext_write_extent() that sounds like not
good logic. I believe we need more serious refactoring of hfs_extend_file() logic.
Potentially, hfs_extend_file() can check that we have HFS_FLG_EXT_DIRTY and
execute logic of write extent without calling himself again. But I haven’t clear picture
of necessary refactoring efforts yet.
Thanks,
Slava.
syzbot suspects this issue was fixed by commit:
commit 6f861765464f43a71462d52026fbddfc858239a5
Author: Jan Kara <[email protected]>
Date: Wed Nov 1 17:43:10 2023 +0000
fs: Block writes to mounted block devices
bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=12bedb0c180000
start commit: 610a9b8f49fb Linux 6.7-rc8
git tree: upstream
kernel config: https://syzkaller.appspot.com/x/.config?x=247b5a935d307ee5
dashboard link: https://syzkaller.appspot.com/bug?extid=41a88b825a315aac2254
syz repro: https://syzkaller.appspot.com/x/repro.syz?x=1552fe19e80000
C reproducer: https://syzkaller.appspot.com/x/repro.c?x=1419bcade80000
If the result looks correct, please mark the issue as fixed by replying with:
#syz fix: fs: Block writes to mounted block devices
For information about bisection process see: https://goo.gl/tpsmEJ#bisection
On Wed 21-02-24 20:10:04, syzbot wrote:
> syzbot suspects this issue was fixed by commit:
>
> commit 6f861765464f43a71462d52026fbddfc858239a5
> Author: Jan Kara <[email protected]>
> Date: Wed Nov 1 17:43:10 2023 +0000
>
> fs: Block writes to mounted block devices
>
> bisection log: https://syzkaller.appspot.com/x/bisect.txt?x=12bedb0c180000
> start commit: 610a9b8f49fb Linux 6.7-rc8
> git tree: upstream
> kernel config: https://syzkaller.appspot.com/x/.config?x=247b5a935d307ee5
> dashboard link: https://syzkaller.appspot.com/bug?extid=41a88b825a315aac2254
> syz repro: https://syzkaller.appspot.com/x/repro.syz?x=1552fe19e80000
> C reproducer: https://syzkaller.appspot.com/x/repro.c?x=1419bcade80000
>
> If the result looks correct, please mark the issue as fixed by replying with:
>
> #syz fix: fs: Block writes to mounted block devices
Unlikely. The report seems more like a lockdep annotation problem where
different btrees used in HFS share the same lockdep key.
Honza
--
Jan Kara <[email protected]>
SUSE Labs, CR