[syzbot] [hfs?] possible deadlock in hfs_extend_file (2)

[syzbot] [hfs?] possible deadlock in hfs_extend_file (2)

[syzbot]

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: syzbot+41a88b825a315aac2254@syzkaller.appspotmail.com

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 syzkaller@googlegroups.com.

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

Re: [syzbot] [hfs?] possible deadlock in hfs_extend_file (2)

[syzbot]

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: syzbot+41a88b825a315aac2254@syzkaller.appspotmail.com

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.

[PATCH] hfs: fix deadlock in hfs_extend_file

[Edward Adam Davis]

[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: syzbot+41a88b825a315aac2254@syzkaller.appspotmail.com
Signed-off-by: Edward Adam Davis <eadavis@qq.com>
---
 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

Re: [syzbot] [hfs?] possible deadlock in hfs_extend_file (2)

[syzbot]

syzbot has bisected this issue to:

commit 54640c7502e5ed41fbf4eedd499e85f9acc9698f
Author: Ernesto A. Fernández <ernesto.mnd.fernandez@gmail.com>
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: syzbot+41a88b825a315aac2254@syzkaller.appspotmail.com
Fixes: 54640c7502e5 ("hfs: prevent btree data loss on ENOSPC")

For information about bisection process see: https://goo.gl/tpsmEJ#bisection

Re: [PATCH] hfs: fix deadlock in hfs_extend_file

[Matthew Wilcox]

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: syzbot+41a88b825a315aac2254@syzkaller.appspotmail.com
> Signed-off-by: Edward Adam Davis <eadavis@qq.com>
> ---
>  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
> 
> 

Re: [PATCH] hfs: fix deadlock in hfs_extend_file

[Viacheslav Dubeyko]

> On Jan 2, 2024, at 3:36 PM, Edward Adam Davis <eadavis@qq.com> 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: syzbot+41a88b825a315aac2254@syzkaller.appspotmail.com
> Signed-off-by: Edward Adam Davis <eadavis@qq.com>
> ---
> 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.

Re: [syzbot] [hfs?] possible deadlock in hfs_extend_file (2)

[syzbot]

syzbot suspects this issue was fixed by commit:

commit 6f861765464f43a71462d52026fbddfc858239a5
Author: Jan Kara <jack@suse.cz>
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

Re: [syzbot] [hfs?] possible deadlock in hfs_extend_file (2)

[Jan Kara]

On Wed 21-02-24 20:10:04, syzbot wrote:
> syzbot suspects this issue was fixed by commit:
> 
> commit 6f861765464f43a71462d52026fbddfc858239a5
> Author: Jan Kara <jack@suse.cz>
> 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 <jack@suse.com>
SUSE Labs, CR