Re: [syzbot] [reiserfs?] possible deadlock in open_xa_dir

Re: [syzbot] [reiserfs?] possible deadlock in open_xa_dir

[syzbot]

syzbot has found a reproducer for the following issue on:

HEAD commit:    78b421b6a7c6 Merge tag 'linux-watchdog-6.4-rc1' of git://w..
git tree:       upstream
console+strace: https://syzkaller.appspot.com/x/log.txt?x=150c42a8280000
kernel config:  https://syzkaller.appspot.com/x/.config?x=73a06f6ef2d5b492
dashboard link: https://syzkaller.appspot.com/bug?extid=8fb64a61fdd96b50f3b8
compiler:       Debian clang version 15.0.7, GNU ld (GNU Binutils for Debian) 2.35.2
syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=14e2e518280000
C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=17c03112280000

Downloadable assets:
disk image: https://storage.googleapis.com/syzbot-assets/25168a5d3be8/disk-78b421b6.raw.xz
vmlinux: https://storage.googleapis.com/syzbot-assets/f5af9f784d9d/vmlinux-78b421b6.xz
kernel image: https://storage.googleapis.com/syzbot-assets/7781e4e3e606/bzImage-78b421b6.xz
mounted in repro: https://storage.googleapis.com/syzbot-assets/29a59b19c7a8/mount_0.gz

IMPORTANT: if you fix the issue, please add the following tag to the commit:
Reported-by: syzbot+8fb64a61fdd96b50f3b8@syzkaller.appspotmail.com

======================================================
WARNING: possible circular locking dependency detected
6.3.0-syzkaller-13164-g78b421b6a7c6 #0 Not tainted
------------------------------------------------------
syz-executor363/4996 is trying to acquire lock:
ffff888074558980 (&type->i_mutex_dir_key#6/3){+.+.}-{3:3}, at: inode_lock_nested include/linux/fs.h:810 [inline]
ffff888074558980 (&type->i_mutex_dir_key#6/3){+.+.}-{3:3}, at: open_xa_root fs/reiserfs/xattr.c:128 [inline]
ffff888074558980 (&type->i_mutex_dir_key#6/3){+.+.}-{3:3}, at: open_xa_dir+0x136/0x610 fs/reiserfs/xattr.c:153

but task is already holding lock:
ffff8880156f2090 (&sbi->lock){+.+.}-{3:3}, at: reiserfs_write_lock_nested+0x5f/0xd0 fs/reiserfs/lock.c:78

which lock already depends on the new lock.


the existing dependency chain (in reverse order) is:

-> #1 (&sbi->lock){+.+.}-{3:3}:
       lock_acquire+0x1e3/0x520 kernel/locking/lockdep.c:5691
       __mutex_lock_common+0x1d8/0x2530 kernel/locking/mutex.c:603
       __mutex_lock kernel/locking/mutex.c:747 [inline]
       mutex_lock_nested+0x1b/0x20 kernel/locking/mutex.c:799
       reiserfs_write_lock+0x7a/0xd0 fs/reiserfs/lock.c:27
       reiserfs_mkdir+0x321/0x8f0 fs/reiserfs/namei.c:829
       xattr_mkdir fs/reiserfs/xattr.c:77 [inline]
       open_xa_root fs/reiserfs/xattr.c:137 [inline]
       open_xa_dir+0x2cd/0x610 fs/reiserfs/xattr.c:153
       xattr_lookup+0x24/0x280 fs/reiserfs/xattr.c:396
       reiserfs_xattr_set_handle+0xfc/0xdc0 fs/reiserfs/xattr.c:534
       reiserfs_security_write+0x157/0x1d0 fs/reiserfs/xattr_security.c:106
       reiserfs_new_inode+0x1631/0x1d40 fs/reiserfs/inode.c:2111
       reiserfs_create+0x3e7/0x6e0 fs/reiserfs/namei.c:666
       lookup_open fs/namei.c:3492 [inline]
       open_last_lookups fs/namei.c:3560 [inline]
       path_openat+0x13df/0x3170 fs/namei.c:3788
       do_filp_open+0x234/0x490 fs/namei.c:3818
       do_sys_openat2+0x13f/0x500 fs/open.c:1356
       do_sys_open fs/open.c:1372 [inline]
       __do_sys_openat fs/open.c:1388 [inline]
       __se_sys_openat fs/open.c:1383 [inline]
       __x64_sys_openat+0x247/0x290 fs/open.c:1383
       do_syscall_x64 arch/x86/entry/common.c:50 [inline]
       do_syscall_64+0x41/0xc0 arch/x86/entry/common.c:80
       entry_SYSCALL_64_after_hwframe+0x63/0xcd

-> #0 (&type->i_mutex_dir_key#6/3){+.+.}-{3:3}:
       check_prev_add kernel/locking/lockdep.c:3108 [inline]
       check_prevs_add kernel/locking/lockdep.c:3227 [inline]
       validate_chain+0x166b/0x58e0 kernel/locking/lockdep.c:3842
       __lock_acquire+0x1295/0x2000 kernel/locking/lockdep.c:5074
       lock_acquire+0x1e3/0x520 kernel/locking/lockdep.c:5691
       down_write_nested+0x3d/0x50 kernel/locking/rwsem.c:1689
       inode_lock_nested include/linux/fs.h:810 [inline]
       open_xa_root fs/reiserfs/xattr.c:128 [inline]
       open_xa_dir+0x136/0x610 fs/reiserfs/xattr.c:153
       xattr_lookup+0x24/0x280 fs/reiserfs/xattr.c:396
       reiserfs_xattr_get+0xfd/0x570 fs/reiserfs/xattr.c:678
       __vfs_getxattr+0x436/0x470 fs/xattr.c:424
       smk_fetch+0xb1/0x140 security/smack/smack_lsm.c:295
       smack_d_instantiate+0x6d9/0xb40 security/smack/smack_lsm.c:3475
       security_d_instantiate+0x9b/0xf0 security/security.c:3760
       d_instantiate_new+0x65/0x120 fs/dcache.c:2053
       reiserfs_create+0x5cf/0x6e0 fs/reiserfs/namei.c:692
       lookup_open fs/namei.c:3492 [inline]
       open_last_lookups fs/namei.c:3560 [inline]
       path_openat+0x13df/0x3170 fs/namei.c:3788
       do_filp_open+0x234/0x490 fs/namei.c:3818
       do_sys_openat2+0x13f/0x500 fs/open.c:1356
       do_sys_open fs/open.c:1372 [inline]
       __do_sys_openat fs/open.c:1388 [inline]
       __se_sys_openat fs/open.c:1383 [inline]
       __x64_sys_openat+0x247/0x290 fs/open.c:1383
       do_syscall_x64 arch/x86/entry/common.c:50 [inline]
       do_syscall_64+0x41/0xc0 arch/x86/entry/common.c:80
       entry_SYSCALL_64_after_hwframe+0x63/0xcd

other info that might help us debug this:

 Possible unsafe locking scenario:

       CPU0                    CPU1
       ----                    ----
  lock(&sbi->lock);
                               lock(&type->i_mutex_dir_key#6/3);
                               lock(&sbi->lock);
  lock(&type->i_mutex_dir_key#6/3);

 *** DEADLOCK ***

3 locks held by syz-executor363/4996:
 #0: ffff88807c6be460 (sb_writers#9){.+.+}-{0:0}, at: mnt_want_write+0x3f/0x90 fs/namespace.c:394
 #1: ffff8880745582e0 (&type->i_mutex_dir_key#6){+.+.}-{3:3}, at: inode_lock include/linux/fs.h:775 [inline]
 #1: ffff8880745582e0 (&type->i_mutex_dir_key#6){+.+.}-{3:3}, at: open_last_lookups fs/namei.c:3557 [inline]
 #1: ffff8880745582e0 (&type->i_mutex_dir_key#6){+.+.}-{3:3}, at: path_openat+0x7ba/0x3170 fs/namei.c:3788
 #2: ffff8880156f2090 (&sbi->lock){+.+.}-{3:3}, at: reiserfs_write_lock_nested+0x5f/0xd0 fs/reiserfs/lock.c:78

stack backtrace:
CPU: 1 PID: 4996 Comm: syz-executor363 Not tainted 6.3.0-syzkaller-13164-g78b421b6a7c6 #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 04/14/2023
Call Trace:
 <TASK>
 __dump_stack lib/dump_stack.c:88 [inline]
 dump_stack_lvl+0x1e7/0x2d0 lib/dump_stack.c:106
 check_noncircular+0x2fe/0x3b0 kernel/locking/lockdep.c:2188
 check_prev_add kernel/locking/lockdep.c:3108 [inline]
 check_prevs_add kernel/locking/lockdep.c:3227 [inline]
 validate_chain+0x166b/0x58e0 kernel/locking/lockdep.c:3842
 __lock_acquire+0x1295/0x2000 kernel/locking/lockdep.c:5074
 lock_acquire+0x1e3/0x520 kernel/locking/lockdep.c:5691
 down_write_nested+0x3d/0x50 kernel/locking/rwsem.c:1689
 inode_lock_nested include/linux/fs.h:810 [inline]
 open_xa_root fs/reiserfs/xattr.c:128 [inline]
 open_xa_dir+0x136/0x610 fs/reiserfs/xattr.c:153
 xattr_lookup+0x24/0x280 fs/reiserfs/xattr.c:396
 reiserfs_xattr_get+0xfd/0x570 fs/reiserfs/xattr.c:678
 __vfs_getxattr+0x436/0x470 fs/xattr.c:424
 smk_fetch+0xb1/0x140 security/smack/smack_lsm.c:295
 smack_d_instantiate+0x6d9/0xb40 security/smack/smack_lsm.c:3475
 security_d_instantiate+0x9b/0xf0 security/security.c:3760
 d_instantiate_new+0x65/0x120 fs/dcache.c:2053
 reiserfs_create+0x5cf/0x6e0 fs/reiserfs/namei.c:692
 lookup_open fs/namei.c:3492 [inline]
 open_last_lookups fs/namei.c:3560 [inline]
 path_openat+0x13df/0x3170 fs/namei.c:3788
 do_filp_open+0x234/0x490 fs/namei.c:3818
 do_sys_openat2+0x13f/0x500 fs/open.c:1356
 do_sys_open fs/open.c:1372 [inline]
 __do_sys_openat fs/open.c:1388 [inline]
 __se_sys_openat fs/open.c:1383 [inline]
 __x64_sys_openat+0x247/0x290 fs/open.c:1383
 do_syscall_x64 arch/x86/entry/common.c:50 [inline]
 do_syscall_64+0x41/0xc0 arch/x86/entry/common.c:80
 entry_SYSCALL_64_after_hwframe+0x63/0xcd
RIP: 0033:0x7fdd867933d9
Code: 28 00 00 00 75 05 48 83 c4 28 c3 e8 b1 14 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 b8 ff ff ff f7 d8 64 89 01 48
RSP: 002b:00007fdd867402f8 EFLAGS: 00000246 ORIG_RAX: 0000000000000101
RAX: ffffffffffffffda RBX: 00007fdd868196c0 RCX: 00007fdd867933d9
RDX: 000000000000275a RSI: 0000000020000080 RDI: 00000000ffffff9c


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

Re: [syzbot] [reiserfs?] possible deadlock in open_xa_dir

[syzbot]

syzbot has bisected this issue to:

commit d82dcd9e21b77d338dc4875f3d4111f0db314a7c
Author: Roberto Sassu <roberto.sassu@huawei.com>
Date:   Fri Mar 31 12:32:18 2023 +0000

    reiserfs: Add security prefix to xattr name in reiserfs_security_write()

bisection log:  https://syzkaller.appspot.com/x/bisect.txt?x=14403182280000
start commit:   3c4aa4434377 Merge tag 'ceph-for-6.4-rc1' of https://githu..
git tree:       upstream
final oops:     https://syzkaller.appspot.com/x/report.txt?x=16403182280000
console output: https://syzkaller.appspot.com/x/log.txt?x=12403182280000
kernel config:  https://syzkaller.appspot.com/x/.config?x=73a06f6ef2d5b492
dashboard link: https://syzkaller.appspot.com/bug?extid=8fb64a61fdd96b50f3b8
syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=12442414280000
C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=176a7318280000

Reported-by: syzbot+8fb64a61fdd96b50f3b8@syzkaller.appspotmail.com
Fixes: d82dcd9e21b7 ("reiserfs: Add security prefix to xattr name in reiserfs_security_write()")

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

Re: [syzbot] [reiserfs?] possible deadlock in open_xa_dir

[Paul Moore]

On Fri, May 5, 2023 at 4:51 PM syzbot
<syzbot+8fb64a61fdd96b50f3b8@syzkaller.appspotmail.com> wrote:
>
> syzbot has bisected this issue to:
>
> commit d82dcd9e21b77d338dc4875f3d4111f0db314a7c
> Author: Roberto Sassu <roberto.sassu@huawei.com>
> Date:   Fri Mar 31 12:32:18 2023 +0000
>
>     reiserfs: Add security prefix to xattr name in reiserfs_security_write()
>
> bisection log:  https://syzkaller.appspot.com/x/bisect.txt?x=14403182280000
> start commit:   3c4aa4434377 Merge tag 'ceph-for-6.4-rc1' of https://githu..
> git tree:       upstream
> final oops:     https://syzkaller.appspot.com/x/report.txt?x=16403182280000
> console output: https://syzkaller.appspot.com/x/log.txt?x=12403182280000
> kernel config:  https://syzkaller.appspot.com/x/.config?x=73a06f6ef2d5b492
> dashboard link: https://syzkaller.appspot.com/bug?extid=8fb64a61fdd96b50f3b8
> syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=12442414280000
> C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=176a7318280000
>
> Reported-by: syzbot+8fb64a61fdd96b50f3b8@syzkaller.appspotmail.com
> Fixes: d82dcd9e21b7 ("reiserfs: Add security prefix to xattr name in reiserfs_security_write()")
>
> For information about bisection process see: https://goo.gl/tpsmEJ#bisection

I don't think Roberto's patch identified above is the actual root
cause of this problem as reiserfs_xattr_set_handle() is called in
reiserfs_security_write() both before and after the patch.  However,
due to some bad logic in reiserfs_security_write() which Roberto
corrected, I'm thinking that it is possible this code is being
exercised for the first time and syzbot is starting to trigger a
locking issue in the reiserfs code ... ?

-- 
paul-moore.com

Re: [syzbot] [reiserfs?] possible deadlock in open_xa_dir

[Roberto Sassu]

On 5/5/2023 11:36 PM, Paul Moore wrote:
> On Fri, May 5, 2023 at 4:51 PM syzbot
> <syzbot+8fb64a61fdd96b50f3b8@syzkaller.appspotmail.com> wrote:
>>
>> syzbot has bisected this issue to:
>>
>> commit d82dcd9e21b77d338dc4875f3d4111f0db314a7c
>> Author: Roberto Sassu <roberto.sassu@huawei.com>
>> Date:   Fri Mar 31 12:32:18 2023 +0000
>>
>>      reiserfs: Add security prefix to xattr name in reiserfs_security_write()
>>
>> bisection log:  https://syzkaller.appspot.com/x/bisect.txt?x=14403182280000
>> start commit:   3c4aa4434377 Merge tag 'ceph-for-6.4-rc1' of https://githu..
>> git tree:       upstream
>> final oops:     https://syzkaller.appspot.com/x/report.txt?x=16403182280000
>> console output: https://syzkaller.appspot.com/x/log.txt?x=12403182280000
>> kernel config:  https://syzkaller.appspot.com/x/.config?x=73a06f6ef2d5b492
>> dashboard link: https://syzkaller.appspot.com/bug?extid=8fb64a61fdd96b50f3b8
>> syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=12442414280000
>> C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=176a7318280000
>>
>> Reported-by: syzbot+8fb64a61fdd96b50f3b8@syzkaller.appspotmail.com
>> Fixes: d82dcd9e21b7 ("reiserfs: Add security prefix to xattr name in reiserfs_security_write()")
>>
>> For information about bisection process see: https://goo.gl/tpsmEJ#bisection
> 
> I don't think Roberto's patch identified above is the actual root
> cause of this problem as reiserfs_xattr_set_handle() is called in
> reiserfs_security_write() both before and after the patch.  However,
> due to some bad logic in reiserfs_security_write() which Roberto
> corrected, I'm thinking that it is possible this code is being
> exercised for the first time and syzbot is starting to trigger a
> locking issue in the reiserfs code ... ?

+ Jan, Jeff (which basically restructured the lock)

+ Petr, Ingo, Will

I involve the lockdep experts, to get a bit of help on this.

First of all, the lockdep warning is trivial to reproduce:

# dd if=/dev/zero of=reiserfs.img bs=1M count=100
# losetup -f --show reiserfs.img
/dev/loop0
# mkfs.reiserfs /dev/loop0
# mount /dev/loop0 /mnt/
# touch file0

In the testing system, Smack is the major LSM.

Ok, so the warning here is clear:

https://syzkaller.appspot.com/x/log.txt?x=12403182280000

However, I was looking if that can really happen. From this:

[   77.746561][ T5418] -> #1 (&sbi->lock){+.+.}-{3:3}:
[   77.753772][ T5418]        lock_acquire+0x23e/0x630
[   77.758792][ T5418]        __mutex_lock_common+0x1d8/0x2530
[   77.764504][ T5418]        mutex_lock_nested+0x1b/0x20
[   77.769868][ T5418]        reiserfs_write_lock+0x70/0xc0
[   77.775321][ T5418]        reiserfs_mkdir+0x321/0x870

I see that the lock is taken in reiserfs_write_lock(), while lockdep says:

[   77.710227][ T5418] but task is already holding lock:
[   77.717587][ T5418] ffff88807568d090 (&sbi->lock){+.+.}-{3:3}, at: 
reiserfs_write_lock_nested+0x4a/0xb0

which is in a different place, I believe here:

int reiserfs_paste_into_item(struct reiserfs_transaction_handle *th,
                              /* Path to the pasted item. */
[...]

         depth = reiserfs_write_unlock_nested(sb);
         dquot_free_space_nodirty(inode, pasted_size);
         reiserfs_write_lock_nested(sb, depth);
         return retval;
}

This is called by reiserfs_add_entry(), which is called by 
reiserfs_create() (it is in the lockdep trace). After returning to 
reiserfs_create(), d_instantiate_new() is called.

I don't know exactly, I take the part that the lock is held. But if it 
is held, how d_instantiate_new() can be executed in another task?

static int reiserfs_create(struct mnt_idmap *idmap, struct inode *dir,
                         struct dentry *dentry, umode_t mode, bool excl)
{

[...]

         reiserfs_write_lock(dir->i_sb);

         retval = journal_begin(&th, dir->i_sb, jbegin_count);

[...]

         d_instantiate_new(dentry, inode);
         retval = journal_end(&th);

out_failed:
         reiserfs_write_unlock(dir->i_sb);

If the lock is held, the scenario lockdep describes cannot happen. Any 
thoughts?

Thanks

Roberto

Re: [syzbot] [reiserfs?] possible deadlock in open_xa_dir

[Roberto Sassu]

On 5/31/2023 11:49 AM, Roberto Sassu wrote:
> On 5/5/2023 11:36 PM, Paul Moore wrote:
>> On Fri, May 5, 2023 at 4:51 PM syzbot
>> <syzbot+8fb64a61fdd96b50f3b8@syzkaller.appspotmail.com> wrote:
>>>
>>> syzbot has bisected this issue to:
>>>
>>> commit d82dcd9e21b77d338dc4875f3d4111f0db314a7c
>>> Author: Roberto Sassu <roberto.sassu@huawei.com>
>>> Date:   Fri Mar 31 12:32:18 2023 +0000
>>>
>>>      reiserfs: Add security prefix to xattr name in 
>>> reiserfs_security_write()
>>>
>>> bisection log:  
>>> https://syzkaller.appspot.com/x/bisect.txt?x=14403182280000
>>> start commit:   3c4aa4434377 Merge tag 'ceph-for-6.4-rc1' of 
>>> https://githu..
>>> git tree:       upstream
>>> final oops:     
>>> https://syzkaller.appspot.com/x/report.txt?x=16403182280000
>>> console output: https://syzkaller.appspot.com/x/log.txt?x=12403182280000
>>> kernel config:  
>>> https://syzkaller.appspot.com/x/.config?x=73a06f6ef2d5b492
>>> dashboard link: 
>>> https://syzkaller.appspot.com/bug?extid=8fb64a61fdd96b50f3b8
>>> syz repro:      
>>> https://syzkaller.appspot.com/x/repro.syz?x=12442414280000
>>> C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=176a7318280000
>>>
>>> Reported-by: syzbot+8fb64a61fdd96b50f3b8@syzkaller.appspotmail.com
>>> Fixes: d82dcd9e21b7 ("reiserfs: Add security prefix to xattr name in 
>>> reiserfs_security_write()")
>>>
>>> For information about bisection process see: 
>>> https://goo.gl/tpsmEJ#bisection
>>
>> I don't think Roberto's patch identified above is the actual root
>> cause of this problem as reiserfs_xattr_set_handle() is called in
>> reiserfs_security_write() both before and after the patch.  However,
>> due to some bad logic in reiserfs_security_write() which Roberto
>> corrected, I'm thinking that it is possible this code is being
>> exercised for the first time and syzbot is starting to trigger a
>> locking issue in the reiserfs code ... ?
> 
> + Jan, Jeff (which basically restructured the lock)

Actually adding Jan and Jeff.

Roberto

> + Petr, Ingo, Will
> 
> I involve the lockdep experts, to get a bit of help on this.
> 
> First of all, the lockdep warning is trivial to reproduce:
> 
> # dd if=/dev/zero of=reiserfs.img bs=1M count=100
> # losetup -f --show reiserfs.img
> /dev/loop0
> # mkfs.reiserfs /dev/loop0
> # mount /dev/loop0 /mnt/
> # touch file0
> 
> In the testing system, Smack is the major LSM.
> 
> Ok, so the warning here is clear:
> 
> https://syzkaller.appspot.com/x/log.txt?x=12403182280000
> 
> However, I was looking if that can really happen. From this:
> 
> [   77.746561][ T5418] -> #1 (&sbi->lock){+.+.}-{3:3}:
> [   77.753772][ T5418]        lock_acquire+0x23e/0x630
> [   77.758792][ T5418]        __mutex_lock_common+0x1d8/0x2530
> [   77.764504][ T5418]        mutex_lock_nested+0x1b/0x20
> [   77.769868][ T5418]        reiserfs_write_lock+0x70/0xc0
> [   77.775321][ T5418]        reiserfs_mkdir+0x321/0x870
> 
> I see that the lock is taken in reiserfs_write_lock(), while lockdep says:
> 
> [   77.710227][ T5418] but task is already holding lock:
> [   77.717587][ T5418] ffff88807568d090 (&sbi->lock){+.+.}-{3:3}, at: 
> reiserfs_write_lock_nested+0x4a/0xb0
> 
> which is in a different place, I believe here:
> 
> int reiserfs_paste_into_item(struct reiserfs_transaction_handle *th,
>                               /* Path to the pasted item. */
> [...]
> 
>          depth = reiserfs_write_unlock_nested(sb);
>          dquot_free_space_nodirty(inode, pasted_size);
>          reiserfs_write_lock_nested(sb, depth);
>          return retval;
> }
> 
> This is called by reiserfs_add_entry(), which is called by 
> reiserfs_create() (it is in the lockdep trace). After returning to 
> reiserfs_create(), d_instantiate_new() is called.
> 
> I don't know exactly, I take the part that the lock is held. But if it 
> is held, how d_instantiate_new() can be executed in another task?
> 
> static int reiserfs_create(struct mnt_idmap *idmap, struct inode *dir,
>                          struct dentry *dentry, umode_t mode, bool excl)
> {
> 
> [...]
> 
>          reiserfs_write_lock(dir->i_sb);
> 
>          retval = journal_begin(&th, dir->i_sb, jbegin_count);
> 
> [...]
> 
>          d_instantiate_new(dentry, inode);
>          retval = journal_end(&th);
> 
> out_failed:
>          reiserfs_write_unlock(dir->i_sb);
> 
> If the lock is held, the scenario lockdep describes cannot happen. Any 
> thoughts?
> 
> Thanks
> 
> Roberto

Re: [syzbot] [reiserfs?] possible deadlock in open_xa_dir

[Jeff Mahoney]

On 5/31/23 05:49, Roberto Sassu wrote:
> On 5/5/2023 11:36 PM, Paul Moore wrote:
>> On Fri, May 5, 2023 at 4:51 PM syzbot
>> <syzbot+8fb64a61fdd96b50f3b8@syzkaller.appspotmail.com> wrote:
>>>
>>> syzbot has bisected this issue to:
>>>
>>> commit d82dcd9e21b77d338dc4875f3d4111f0db314a7c
>>> Author: Roberto Sassu <roberto.sassu@huawei.com>
>>> Date:   Fri Mar 31 12:32:18 2023 +0000
>>>
>>>      reiserfs: Add security prefix to xattr name in 
>>> reiserfs_security_write()
>>>
>>> bisection log:  
>>> https://syzkaller.appspot.com/x/bisect.txt?x=14403182280000
>>> start commit:   3c4aa4434377 Merge tag 'ceph-for-6.4-rc1' of 
>>> https://githu..
>>> git tree:       upstream
>>> final oops:     
>>> https://syzkaller.appspot.com/x/report.txt?x=16403182280000
>>> console output: https://syzkaller.appspot.com/x/log.txt?x=12403182280000
>>> kernel config:  
>>> https://syzkaller.appspot.com/x/.config?x=73a06f6ef2d5b492
>>> dashboard link: 
>>> https://syzkaller.appspot.com/bug?extid=8fb64a61fdd96b50f3b8
>>> syz repro:      
>>> https://syzkaller.appspot.com/x/repro.syz?x=12442414280000
>>> C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=176a7318280000
>>>
>>> Reported-by: syzbot+8fb64a61fdd96b50f3b8@syzkaller.appspotmail.com
>>> Fixes: d82dcd9e21b7 ("reiserfs: Add security prefix to xattr name in 
>>> reiserfs_security_write()")
>>>
>>> For information about bisection process see: 
>>> https://goo.gl/tpsmEJ#bisection
>>
>> I don't think Roberto's patch identified above is the actual root
>> cause of this problem as reiserfs_xattr_set_handle() is called in
>> reiserfs_security_write() both before and after the patch.  However,
>> due to some bad logic in reiserfs_security_write() which Roberto
>> corrected, I'm thinking that it is possible this code is being
>> exercised for the first time and syzbot is starting to trigger a
>> locking issue in the reiserfs code ... ?
> 
> + Jan, Jeff (which basically restructured the lock)
> 
> + Petr, Ingo, Will
> 
> I involve the lockdep experts, to get a bit of help on this.

Yep, looks like that's been broken since it was added in 2009.  Since 
there can't be any users of it, it'd make sense to drop the security 
xattr support from reiserfs entirely.

> First of all, the lockdep warning is trivial to reproduce:
> 
> # dd if=/dev/zero of=reiserfs.img bs=1M count=100
> # losetup -f --show reiserfs.img
> /dev/loop0
> # mkfs.reiserfs /dev/loop0
> # mount /dev/loop0 /mnt/
> # touch file0
> 
> In the testing system, Smack is the major LSM.
> 
> Ok, so the warning here is clear:
> 
> https://syzkaller.appspot.com/x/log.txt?x=12403182280000
> 
> However, I was looking if that can really happen. From this:
> 
> [   77.746561][ T5418] -> #1 (&sbi->lock){+.+.}-{3:3}:
> [   77.753772][ T5418]        lock_acquire+0x23e/0x630
> [   77.758792][ T5418]        __mutex_lock_common+0x1d8/0x2530
> [   77.764504][ T5418]        mutex_lock_nested+0x1b/0x20
> [   77.769868][ T5418]        reiserfs_write_lock+0x70/0xc0
> [   77.775321][ T5418]        reiserfs_mkdir+0x321/0x870
> 
> I see that the lock is taken in reiserfs_write_lock(), while lockdep says:
> 
> [   77.710227][ T5418] but task is already holding lock:
> [   77.717587][ T5418] ffff88807568d090 (&sbi->lock){+.+.}-{3:3}, at: 
> reiserfs_write_lock_nested+0x4a/0xb0
> 
> which is in a different place, I believe here:
> 
> int reiserfs_paste_into_item(struct reiserfs_transaction_handle *th,
>                               /* Path to the pasted item. */
> [...]
> 
>          depth = reiserfs_write_unlock_nested(sb);
>          dquot_free_space_nodirty(inode, pasted_size);
>          reiserfs_write_lock_nested(sb, depth);
>          return retval;
> }
> 
> This is called by reiserfs_add_entry(), which is called by 
> reiserfs_create() (it is in the lockdep trace). After returning to 
> reiserfs_create(), d_instantiate_new() is called.
> 
> I don't know exactly, I take the part that the lock is held. But if it 
> is held, how d_instantiate_new() can be executed in another task?
> 
> static int reiserfs_create(struct mnt_idmap *idmap, struct inode *dir,
>                          struct dentry *dentry, umode_t mode, bool excl)
> {
> 
> [...]
> 
>          reiserfs_write_lock(dir->i_sb);
> 
>          retval = journal_begin(&th, dir->i_sb, jbegin_count);
> 
> [...]
> 
>          d_instantiate_new(dentry, inode);
>          retval = journal_end(&th);
> 
> out_failed:
>          reiserfs_write_unlock(dir->i_sb);
> 
> If the lock is held, the scenario lockdep describes cannot happen. Any 
> thoughts?

It's important to understand that the reiserfs write lock was added as a 
subsystem-specific replacement for the BKL.  Given that reiserfs was 
dying already back then, it made more sense from a time management 
perspective to emulate that behavior internally rather than use new 
locking when practically nobody cared anymore.

See reiserfs_write_unlock_nested and reiserfs_write_lock_nested paired 
throughout the code.  It drops the lock when it passes a point where 
it's likely to schedule, just like the BKL would have.

Yes, it's a mess.  Just let it die quietly.

-Jeff

-- 
Jeff Mahoney
VP Engineering, Linux Systems

Re: [syzbot] [reiserfs?] possible deadlock in open_xa_dir

[Roberto Sassu]

On Thu, 2023-06-01 at 17:22 -0400, Jeff Mahoney wrote:
> On 5/31/23 05:49, Roberto Sassu wrote:
> > On 5/5/2023 11:36 PM, Paul Moore wrote:
> > > On Fri, May 5, 2023 at 4:51 PM syzbot
> > > <syzbot+8fb64a61fdd96b50f3b8@syzkaller.appspotmail.com> wrote:
> > > > syzbot has bisected this issue to:
> > > > 
> > > > commit d82dcd9e21b77d338dc4875f3d4111f0db314a7c
> > > > Author: Roberto Sassu <roberto.sassu@huawei.com>
> > > > Date:   Fri Mar 31 12:32:18 2023 +0000
> > > > 
> > > >      reiserfs: Add security prefix to xattr name in 
> > > > reiserfs_security_write()
> > > > 
> > > > bisection log:  
> > > > https://syzkaller.appspot.com/x/bisect.txt?x=14403182280000
> > > > start commit:   3c4aa4434377 Merge tag 'ceph-for-6.4-rc1' of 
> > > > https://githu..
> > > > git tree:       upstream
> > > > final oops:     
> > > > https://syzkaller.appspot.com/x/report.txt?x=16403182280000
> > > > console output: https://syzkaller.appspot.com/x/log.txt?x=12403182280000
> > > > kernel config:  
> > > > https://syzkaller.appspot.com/x/.config?x=73a06f6ef2d5b492
> > > > dashboard link: 
> > > > https://syzkaller.appspot.com/bug?extid=8fb64a61fdd96b50f3b8
> > > > syz repro:      
> > > > https://syzkaller.appspot.com/x/repro.syz?x=12442414280000
> > > > C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=176a7318280000
> > > > 
> > > > Reported-by: syzbot+8fb64a61fdd96b50f3b8@syzkaller.appspotmail.com
> > > > Fixes: d82dcd9e21b7 ("reiserfs: Add security prefix to xattr name in 
> > > > reiserfs_security_write()")
> > > > 
> > > > For information about bisection process see: 
> > > > https://goo.gl/tpsmEJ#bisection
> > > 
> > > I don't think Roberto's patch identified above is the actual root
> > > cause of this problem as reiserfs_xattr_set_handle() is called in
> > > reiserfs_security_write() both before and after the patch.  However,
> > > due to some bad logic in reiserfs_security_write() which Roberto
> > > corrected, I'm thinking that it is possible this code is being
> > > exercised for the first time and syzbot is starting to trigger a
> > > locking issue in the reiserfs code ... ?
> > 
> > + Jan, Jeff (which basically restructured the lock)
> > 
> > + Petr, Ingo, Will

Peter, clearly (sorry!)

> I involve the lockdep experts, to get a bit of help on this.
> 
> Yep, looks like that's been broken since it was added in 2009.  Since 
> there can't be any users of it, it'd make sense to drop the security 
> xattr support from reiserfs entirely.

Thanks, Jeff. Will make a patch to implement your suggestion.

Meanwhile, I learned how to read lockdep a bit better. The following
format could have helped me to understand it more quickly. The proposal
is simply to change #n to CPU#n at the top of the trace, define labels
L#n for the locks, and add them where effectively are held.

[   77.746561][ T5418] -> CPU1 (&sbi->lock){+.+.}-{3:3}:
[   77.753772][ T5418]        lock_acquire+0x23e/0x630
[   77.758792][ T5418]        __mutex_lock_common+0x1d8/0x2530
[   77.764504][ T5418]   (L3) mutex_lock_nested+0x1b/0x20
[   77.769868][ T5418]        reiserfs_write_lock+0x70/0xc0
[   77.775321][ T5418]        reiserfs_mkdir+0x321/0x870
[   77.780509][ T5418]   (L2) open_xa_dir+0x259/0x540
[   77.785440][ T5418]        xattr_lookup+0x17/0x210
[   77.790378][ T5418]        reiserfs_xattr_set_handle+0xda/0xc80
[   77.796448][ T5418]        reiserfs_security_write+0x134/0x190
[   77.802416][ T5418]        reiserfs_new_inode+0x13bf/0x1a90
[   77.808124][ T5418]        reiserfs_create+0x3b1/0x680
[   77.813399][ T5418]        path_openat+0xf1e/0x2c10
[   77.818415][ T5418]        do_filp_open+0x22a/0x440
[   77.823433][ T5418]        do_sys_openat2+0x10f/0x430
[   77.828624][ T5418]        __x64_sys_creat+0x11e/0x160
[   77.833905][ T5418]        do_syscall_64+0x41/0xc0
[   77.838926][ T5418]        entry_SYSCALL_64_after_hwframe+0x63/0xcd
[   77.845514][ T5418] 
[   77.845514][ T5418] -> CPU0 (&type->i_mutex_dir_key#8/3){+.+.}-{3:3}:
[   77.854118][ T5418]        validate_chain+0x166b/0x58e0
[   77.859488][ T5418]        __lock_acquire+0x125b/0x1f80
[   77.864853][ T5418]        lock_acquire+0x23e/0x630
[   77.869909][ T5418]   (L4) down_write_nested+0x3d/0x50
[   77.875186][ T5418]        open_xa_dir+0x134/0x540
[   77.880117][ T5418]        xattr_lookup+0x17/0x210
[   77.885050][ T5418]        reiserfs_xattr_get+0xe1/0x4a0
[   77.890501][ T5418]        __vfs_getxattr+0x2fe/0x350
[   77.895802][ T5418]        smk_fetch+0x98/0xf0
[   77.900382][ T5418]        smack_d_instantiate+0x5d5/0xa20
[   77.906018][ T5418]        security_d_instantiate+0x6b/0xb0
[   77.911736][ T5418]        d_instantiate_new+0x5e/0xe0
[   77.917013][ T5418]   (L1) reiserfs_create+0x5ee/0x680
[   77.922293][ T5418]        path_openat+0xf1e/0x2c10
[   77.927308][ T5418]        do_filp_open+0x22a/0x440
[   77.932330][ T5418]        do_sys_openat2+0x10f/0x430
[   77.937515][ T5418]        __x64_sys_creat+0x11e/0x160
[   77.942874][ T5418]        do_syscall_64+0x41/0xc0
[   77.947796][ T5418]        entry_SYSCALL_64_after_hwframe+0x63/0xcd
[   77.954200][ T5418] 
[   77.954200][ T5418] other info that might help us debug this:
[   77.954200][ T5418] 
[   77.964508][ T5418]  Possible unsafe locking scenario:
[   77.964508][ T5418] 
[   77.972034][ T5418]   CPU0                         CPU1
[   77.977394][ T5418]   ----                         ----
[   77.982748][ T5418]   L1: lock(&sbi->lock);
[   77.986726][ T5418]                                L2: lock(&type->i_mutex_dir_key#8/3);
[   77.994618][ T5418]                                L3: lock(&sbi->lock);
[   78.001118][ T5418]   L4: lock(&type->i_mutex_dir_key#8/3);

Thanks

Roberto

> > First of all, the lockdep warning is trivial to reproduce:
> > 
> > # dd if=/dev/zero of=reiserfs.img bs=1M count=100
> > # losetup -f --show reiserfs.img
> > /dev/loop0
> > # mkfs.reiserfs /dev/loop0
> > # mount /dev/loop0 /mnt/
> > # touch file0
> > 
> > In the testing system, Smack is the major LSM.
> > 
> > Ok, so the warning here is clear:
> > 
> > https://syzkaller.appspot.com/x/log.txt?x=12403182280000
> > 
> > However, I was looking if that can really happen. From this:
> > 
> > [   77.746561][ T5418] -> #1 (&sbi->lock){+.+.}-{3:3}:
> > [   77.753772][ T5418]        lock_acquire+0x23e/0x630
> > [   77.758792][ T5418]        __mutex_lock_common+0x1d8/0x2530
> > [   77.764504][ T5418]        mutex_lock_nested+0x1b/0x20
> > [   77.769868][ T5418]        reiserfs_write_lock+0x70/0xc0
> > [   77.775321][ T5418]        reiserfs_mkdir+0x321/0x870
> > 
> > I see that the lock is taken in reiserfs_write_lock(), while lockdep says:
> > 
> > [   77.710227][ T5418] but task is already holding lock:
> > [   77.717587][ T5418] ffff88807568d090 (&sbi->lock){+.+.}-{3:3}, at: 
> > reiserfs_write_lock_nested+0x4a/0xb0
> > 
> > which is in a different place, I believe here:
> > 
> > int reiserfs_paste_into_item(struct reiserfs_transaction_handle *th,
> >                               /* Path to the pasted item. */
> > [...]
> > 
> >          depth = reiserfs_write_unlock_nested(sb);
> >          dquot_free_space_nodirty(inode, pasted_size);
> >          reiserfs_write_lock_nested(sb, depth);
> >          return retval;
> > }
> > 
> > This is called by reiserfs_add_entry(), which is called by 
> > reiserfs_create() (it is in the lockdep trace). After returning to 
> > reiserfs_create(), d_instantiate_new() is called.
> > 
> > I don't know exactly, I take the part that the lock is held. But if it 
> > is held, how d_instantiate_new() can be executed in another task?
> > 
> > static int reiserfs_create(struct mnt_idmap *idmap, struct inode *dir,
> >                          struct dentry *dentry, umode_t mode, bool excl)
> > {
> > 
> > [...]
> > 
> >          reiserfs_write_lock(dir->i_sb);
> > 
> >          retval = journal_begin(&th, dir->i_sb, jbegin_count);
> > 
> > [...]
> > 
> >          d_instantiate_new(dentry, inode);
> >          retval = journal_end(&th);
> > 
> > out_failed:
> >          reiserfs_write_unlock(dir->i_sb);
> > 
> > If the lock is held, the scenario lockdep describes cannot happen. Any 
> > thoughts?
> 
> It's important to understand that the reiserfs write lock was added as a 
> subsystem-specific replacement for the BKL.  Given that reiserfs was 
> dying already back then, it made more sense from a time management 
> perspective to emulate that behavior internally rather than use new 
> locking when practically nobody cared anymore.
> 
> See reiserfs_write_unlock_nested and reiserfs_write_lock_nested paired 
> throughout the code.  It drops the lock when it passes a point where 
> it's likely to schedule, just like the BKL would have.
> 
> Yes, it's a mess.  Just let it die quietly.
> 
> -Jeff
> 

Re: [syzbot] [reiserfs?] possible deadlock in open_xa_dir

[Roberto Sassu]

[-- Attachment #1: Type: text/plain, Size: 2798 bytes --]

On Fri, 2023-06-02 at 09:20 +0200, Roberto Sassu wrote:
> On Thu, 2023-06-01 at 17:22 -0400, Jeff Mahoney wrote:
> > On 5/31/23 05:49, Roberto Sassu wrote:
> > > On 5/5/2023 11:36 PM, Paul Moore wrote:
> > > > On Fri, May 5, 2023 at 4:51 PM syzbot
> > > > <syzbot+8fb64a61fdd96b50f3b8@syzkaller.appspotmail.com> wrote:
> > > > > syzbot has bisected this issue to:
> > > > > 
> > > > > commit d82dcd9e21b77d338dc4875f3d4111f0db314a7c
> > > > > Author: Roberto Sassu <roberto.sassu@huawei.com>
> > > > > Date:   Fri Mar 31 12:32:18 2023 +0000
> > > > > 
> > > > >      reiserfs: Add security prefix to xattr name in 
> > > > > reiserfs_security_write()
> > > > > 
> > > > > bisection log:  
> > > > > https://syzkaller.appspot.com/x/bisect.txt?x=14403182280000
> > > > > start commit:   3c4aa4434377 Merge tag 'ceph-for-6.4-rc1' of 
> > > > > https://githu..
> > > > > git tree:       upstream
> > > > > final oops:     
> > > > > https://syzkaller.appspot.com/x/report.txt?x=16403182280000
> > > > > console output: https://syzkaller.appspot.com/x/log.txt?x=12403182280000
> > > > > kernel config:  
> > > > > https://syzkaller.appspot.com/x/.config?x=73a06f6ef2d5b492
> > > > > dashboard link: 
> > > > > https://syzkaller.appspot.com/bug?extid=8fb64a61fdd96b50f3b8
> > > > > syz repro:      
> > > > > https://syzkaller.appspot.com/x/repro.syz?x=12442414280000
> > > > > C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=176a7318280000
> > > > > 
> > > > > Reported-by: syzbot+8fb64a61fdd96b50f3b8@syzkaller.appspotmail.com
> > > > > Fixes: d82dcd9e21b7 ("reiserfs: Add security prefix to xattr name in 
> > > > > reiserfs_security_write()")
> > > > > 
> > > > > For information about bisection process see: 
> > > > > https://goo.gl/tpsmEJ#bisection
> > > > 
> > > > I don't think Roberto's patch identified above is the actual root
> > > > cause of this problem as reiserfs_xattr_set_handle() is called in
> > > > reiserfs_security_write() both before and after the patch.  However,
> > > > due to some bad logic in reiserfs_security_write() which Roberto
> > > > corrected, I'm thinking that it is possible this code is being
> > > > exercised for the first time and syzbot is starting to trigger a
> > > > locking issue in the reiserfs code ... ?
> > > 
> > > + Jan, Jeff (which basically restructured the lock)
> > > 
> > > + Petr, Ingo, Will
> 
> Peter, clearly (sorry!)
> 
> > I involve the lockdep experts, to get a bit of help on this.
> > 
> > Yep, looks like that's been broken since it was added in 2009.  Since 
> > there can't be any users of it, it'd make sense to drop the security 
> > xattr support from reiserfs entirely.
> 
> Thanks, Jeff. Will make a patch to implement your suggestion.

#syz test: git://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/lsm.git next

[-- Attachment #2: 0001-reiserfs-Disable-security-xattr-initialization-since.patch --]
[-- Type: text/x-patch, Size: 2210 bytes --]

From a31f5b09e39e5a964457b0a52aa9c437a0bf7f63 Mon Sep 17 00:00:00 2001
From: Roberto Sassu <roberto.sassu@huawei.com>
Date: Fri, 2 Jun 2023 10:10:28 +0200
Subject: [PATCH] reiserfs: Disable security xattr initialization since it
 never worked

Commit d82dcd9e21b7 ("reiserfs: Add security prefix to xattr name in
reiserfs_security_write()"), while fixed the security xattr initialization,
it also revealed a circular locking dependency between the reiserfs write
lock and the inode lock.

Since the bug in security xattr initialization was introduced since the
beginning, there cannot be any user of this feature. Instead of trying to
fix the locking dependency, which was already challenging to convert from
BLK, just disable the feature.

However, still keep the security xattr handler, since it was introduced
earlier, and users might have manually added xattrs.

Reported-by: syzbot+8fb64a61fdd96b50f3b8@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=8fb64a61fdd96b50f3b8
Suggested-by: Jeff Mahoney <jeffm@suse.com>
Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 fs/reiserfs/super.c          | 2 ++
 fs/reiserfs/xattr_security.c | 3 +++
 2 files changed, 5 insertions(+)

diff --git a/fs/reiserfs/super.c b/fs/reiserfs/super.c
index 929acce6e73..262041b87cd 100644
--- a/fs/reiserfs/super.c
+++ b/fs/reiserfs/super.c
@@ -1654,6 +1654,8 @@ static int read_super_block(struct super_block *s, int offset)
 
 	reiserfs_warning(NULL, "", "reiserfs filesystem is deprecated and "
 		"scheduled to be removed from the kernel in 2025");
+	reiserfs_warning(NULL, "", "initializing security xattrs never worked, disable it");
+
 	SB_BUFFER_WITH_SB(s) = bh;
 	SB_DISK_SUPER_BLOCK(s) = rs;
 
diff --git a/fs/reiserfs/xattr_security.c b/fs/reiserfs/xattr_security.c
index 078dd8cc312..4037a998dbf 100644
--- a/fs/reiserfs/xattr_security.c
+++ b/fs/reiserfs/xattr_security.c
@@ -69,6 +69,9 @@ int reiserfs_security_init(struct inode *dir, struct inode *inode,
 	sec->value = NULL;
 	sec->length = 0;
 
+	/* See warning in read_super_block(). */
+	return 0;
+
 	/* Don't add selinux attributes on xattrs - they'll never get used */
 	if (IS_PRIVATE(dir))
 		return 0;
-- 
2.25.1

Re: [syzbot] [reiserfs?] possible deadlock in open_xa_dir

[syzbot]

Hello,

syzbot has tested the proposed patch and the reproducer did not trigger any issue:

Reported-and-tested-by: syzbot+8fb64a61fdd96b50f3b8@syzkaller.appspotmail.com

Tested on:

commit:         4432b507 lsm: fix a number of misspellings
git tree:       git://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/lsm.git next
console output: https://syzkaller.appspot.com/x/log.txt?x=166c541d280000
kernel config:  https://syzkaller.appspot.com/x/.config?x=38526bf24c8d961b
dashboard link: https://syzkaller.appspot.com/bug?extid=8fb64a61fdd96b50f3b8
compiler:       Debian clang version 15.0.7, GNU ld (GNU Binutils for Debian) 2.35.2
patch:          https://syzkaller.appspot.com/x/patch.diff?x=1095cd79280000

Note: testing is done by a robot and is best-effort only.

Re: [syzbot] [reiserfs?] possible deadlock in open_xa_dir

[Roberto Sassu]

[-- Attachment #1: Type: text/plain, Size: 932 bytes --]

On Fri, 2023-06-02 at 02:17 -0700, syzbot wrote:
> Hello,
> 
> syzbot has tested the proposed patch and the reproducer did not trigger any issue:
> 
> Reported-and-tested-by: syzbot+8fb64a61fdd96b50f3b8@syzkaller.appspotmail.com
> 
> Tested on:
> 
> commit:         4432b507 lsm: fix a number of misspellings
> git tree:       git://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/lsm.git next
> console output: https://syzkaller.appspot.com/x/log.txt?x=166c541d280000
> kernel config:  https://syzkaller.appspot.com/x/.config?x=38526bf24c8d961b
> dashboard link: https://syzkaller.appspot.com/bug?extid=8fb64a61fdd96b50f3b8
> compiler:       Debian clang version 15.0.7, GNU ld (GNU Binutils for Debian) 2.35.2
> patch:          https://syzkaller.appspot.com/x/patch.diff?x=1095cd79280000
> 
> Note: testing is done by a robot and is best-effort only.

#syz test: git://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/lsm.git next

[-- Attachment #2: 0001-reiserfs-Disable-by-default-security-xattr-init-sinc.patch --]
[-- Type: text/x-patch, Size: 2884 bytes --]

From 73bb02eb7a751c447af43d7cac7c191329b6dd55 Mon Sep 17 00:00:00 2001
From: Roberto Sassu <roberto.sassu@huawei.com>
Date: Fri, 2 Jun 2023 10:10:28 +0200
Subject: [PATCH] reiserfs: Disable by default security xattr init since it
 never worked

Commit d82dcd9e21b7 ("reiserfs: Add security prefix to xattr name in
reiserfs_security_write()"), while fixed the security xattr initialization,
it also revealed a circular locking dependency between the reiserfs write
lock and the inode lock.

Add the new config option CONFIG_REISERFS_FS_SECURITY_INIT to
enable/disable the feature. Also, since the bug in security xattr
initialization was introduced since the beginning, disable it by default.

Reported-and-tested-by: syzbot+8fb64a61fdd96b50f3b8@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=8fb64a61fdd96b50f3b8
Suggested-by: Jeff Mahoney <jeffm@suse.com>
Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 fs/reiserfs/Kconfig          | 15 +++++++++++++++
 fs/reiserfs/super.c          |  3 +++
 fs/reiserfs/xattr_security.c |  3 +++
 3 files changed, 21 insertions(+)

diff --git a/fs/reiserfs/Kconfig b/fs/reiserfs/Kconfig
index 4d22ecfe0fa..a618d0bda7b 100644
--- a/fs/reiserfs/Kconfig
+++ b/fs/reiserfs/Kconfig
@@ -88,3 +88,18 @@ config REISERFS_FS_SECURITY
 
 	  If you are not using a security module that requires using
 	  extended attributes for file security labels, say N.
+
+config REISERFS_FS_SECURITY_INIT
+	bool "ReiserFS Security Labels initialization"
+	depends on REISERFS_FS_XATTR
+	default false
+	help
+	  Init new inodes with security labels provided by LSMs.
+
+	  It was broken from the beginning, since the xattr name was
+	  missing the 'security.' prefix.
+
+	  Enabling this option might cause lockdep warnings and
+	  ultimately deadlocks.
+
+	  If unsure, say N.
diff --git a/fs/reiserfs/super.c b/fs/reiserfs/super.c
index 929acce6e73..b427d03d0ea 100644
--- a/fs/reiserfs/super.c
+++ b/fs/reiserfs/super.c
@@ -1654,6 +1654,9 @@ static int read_super_block(struct super_block *s, int offset)
 
 	reiserfs_warning(NULL, "", "reiserfs filesystem is deprecated and "
 		"scheduled to be removed from the kernel in 2025");
+	if (IS_ENABLED(CONFIG_REISERFS_FS_SECURITY_INIT))
+		reiserfs_warning(NULL, "", "initializing security xattrs can cause deadlocks");
+
 	SB_BUFFER_WITH_SB(s) = bh;
 	SB_DISK_SUPER_BLOCK(s) = rs;
 
diff --git a/fs/reiserfs/xattr_security.c b/fs/reiserfs/xattr_security.c
index 078dd8cc312..d82c4507803 100644
--- a/fs/reiserfs/xattr_security.c
+++ b/fs/reiserfs/xattr_security.c
@@ -69,6 +69,9 @@ int reiserfs_security_init(struct inode *dir, struct inode *inode,
 	sec->value = NULL;
 	sec->length = 0;
 
+	if (!IS_ENABLED(CONFIG_REISERFS_FS_SECURITY_INIT))
+		return 0;
+
 	/* Don't add selinux attributes on xattrs - they'll never get used */
 	if (IS_PRIVATE(dir))
 		return 0;
-- 
2.25.1

Re: [syzbot] [reiserfs?] possible deadlock in open_xa_dir

[syzbot]

Hello,

syzbot has tested the proposed patch and the reproducer did not trigger any issue:

Reported-and-tested-by: syzbot+8fb64a61fdd96b50f3b8@syzkaller.appspotmail.com

Tested on:

commit:         4432b507 lsm: fix a number of misspellings
git tree:       git://git.kernel.org/pub/scm/linux/kernel/git/pcmoore/lsm.git next
console output: https://syzkaller.appspot.com/x/log.txt?x=16b47dd1280000
kernel config:  https://syzkaller.appspot.com/x/.config?x=178a8c28652084e1
dashboard link: https://syzkaller.appspot.com/bug?extid=8fb64a61fdd96b50f3b8
compiler:       Debian clang version 15.0.7, GNU ld (GNU Binutils for Debian) 2.35.2
patch:          https://syzkaller.appspot.com/x/patch.diff?x=102f6ab6280000

Note: testing is done by a robot and is best-effort only.

Re: [syzbot] [reiserfs?] possible deadlock in open_xa_dir

[Roberto Sassu]

On Fri, 2023-06-02 at 09:20 +0200, Roberto Sassu wrote:
> On Thu, 2023-06-01 at 17:22 -0400, Jeff Mahoney wrote:
> > On 5/31/23 05:49, Roberto Sassu wrote:
> > > On 5/5/2023 11:36 PM, Paul Moore wrote:
> > > > On Fri, May 5, 2023 at 4:51 PM syzbot
> > > > <syzbot+8fb64a61fdd96b50f3b8@syzkaller.appspotmail.com> wrote:
> > > > > syzbot has bisected this issue to:
> > > > > 
> > > > > commit d82dcd9e21b77d338dc4875f3d4111f0db314a7c
> > > > > Author: Roberto Sassu <roberto.sassu@huawei.com>
> > > > > Date:   Fri Mar 31 12:32:18 2023 +0000
> > > > > 
> > > > >      reiserfs: Add security prefix to xattr name in 
> > > > > reiserfs_security_write()
> > > > > 
> > > > > bisection log:  
> > > > > https://syzkaller.appspot.com/x/bisect.txt?x=14403182280000
> > > > > start commit:   3c4aa4434377 Merge tag 'ceph-for-6.4-rc1' of 
> > > > > https://githu..
> > > > > git tree:       upstream
> > > > > final oops:     
> > > > > https://syzkaller.appspot.com/x/report.txt?x=16403182280000
> > > > > console output: https://syzkaller.appspot.com/x/log.txt?x=12403182280000
> > > > > kernel config:  
> > > > > https://syzkaller.appspot.com/x/.config?x=73a06f6ef2d5b492
> > > > > dashboard link: 
> > > > > https://syzkaller.appspot.com/bug?extid=8fb64a61fdd96b50f3b8
> > > > > syz repro:      
> > > > > https://syzkaller.appspot.com/x/repro.syz?x=12442414280000
> > > > > C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=176a7318280000
> > > > > 
> > > > > Reported-by: syzbot+8fb64a61fdd96b50f3b8@syzkaller.appspotmail.com
> > > > > Fixes: d82dcd9e21b7 ("reiserfs: Add security prefix to xattr name in 
> > > > > reiserfs_security_write()")
> > > > > 
> > > > > For information about bisection process see: 
> > > > > https://goo.gl/tpsmEJ#bisection
> > > > 
> > > > I don't think Roberto's patch identified above is the actual root
> > > > cause of this problem as reiserfs_xattr_set_handle() is called in
> > > > reiserfs_security_write() both before and after the patch.  However,
> > > > due to some bad logic in reiserfs_security_write() which Roberto
> > > > corrected, I'm thinking that it is possible this code is being
> > > > exercised for the first time and syzbot is starting to trigger a
> > > > locking issue in the reiserfs code ... ?
> > > 
> > > + Jan, Jeff (which basically restructured the lock)
> > > 
> > > + Petr, Ingo, Will
> 
> Peter, clearly (sorry!)
> 
> > I involve the lockdep experts, to get a bit of help on this.
> > 
> > Yep, looks like that's been broken since it was added in 2009.  Since 
> > there can't be any users of it, it'd make sense to drop the security 
> > xattr support from reiserfs entirely.
> 
> Thanks, Jeff. Will make a patch to implement your suggestion.

Ok, I tried first to disable security xattr initialization and keep the
xattr handler.

Setting the security xattr manually triggers a lockdep warning. Even
worse, setting a trusted xattr manually triggers that too. So, not sure
how we should proceed.

Have you looked at:

https://lore.kernel.org/linux-kernel/8a48ede1-3a45-7c3c-39e9-36001ac09283@huaweicloud.com/

That silences the lockdep warning, but I'm far from saying that it
won't have any side effect...

Thanks

Roberto

> Meanwhile, I learned how to read lockdep a bit better. The following
> format could have helped me to understand it more quickly. The proposal
> is simply to change #n to CPU#n at the top of the trace, define labels
> L#n for the locks, and add them where effectively are held.
> 
> [   77.746561][ T5418] -> CPU1 (&sbi->lock){+.+.}-{3:3}:
> [   77.753772][ T5418]        lock_acquire+0x23e/0x630
> [   77.758792][ T5418]        __mutex_lock_common+0x1d8/0x2530
> [   77.764504][ T5418]   (L3) mutex_lock_nested+0x1b/0x20
> [   77.769868][ T5418]        reiserfs_write_lock+0x70/0xc0
> [   77.775321][ T5418]        reiserfs_mkdir+0x321/0x870
> [   77.780509][ T5418]   (L2) open_xa_dir+0x259/0x540
> [   77.785440][ T5418]        xattr_lookup+0x17/0x210
> [   77.790378][ T5418]        reiserfs_xattr_set_handle+0xda/0xc80
> [   77.796448][ T5418]        reiserfs_security_write+0x134/0x190
> [   77.802416][ T5418]        reiserfs_new_inode+0x13bf/0x1a90
> [   77.808124][ T5418]        reiserfs_create+0x3b1/0x680
> [   77.813399][ T5418]        path_openat+0xf1e/0x2c10
> [   77.818415][ T5418]        do_filp_open+0x22a/0x440
> [   77.823433][ T5418]        do_sys_openat2+0x10f/0x430
> [   77.828624][ T5418]        __x64_sys_creat+0x11e/0x160
> [   77.833905][ T5418]        do_syscall_64+0x41/0xc0
> [   77.838926][ T5418]        entry_SYSCALL_64_after_hwframe+0x63/0xcd
> [   77.845514][ T5418] 
> [   77.845514][ T5418] -> CPU0 (&type->i_mutex_dir_key#8/3){+.+.}-{3:3}:
> [   77.854118][ T5418]        validate_chain+0x166b/0x58e0
> [   77.859488][ T5418]        __lock_acquire+0x125b/0x1f80
> [   77.864853][ T5418]        lock_acquire+0x23e/0x630
> [   77.869909][ T5418]   (L4) down_write_nested+0x3d/0x50
> [   77.875186][ T5418]        open_xa_dir+0x134/0x540
> [   77.880117][ T5418]        xattr_lookup+0x17/0x210
> [   77.885050][ T5418]        reiserfs_xattr_get+0xe1/0x4a0
> [   77.890501][ T5418]        __vfs_getxattr+0x2fe/0x350
> [   77.895802][ T5418]        smk_fetch+0x98/0xf0
> [   77.900382][ T5418]        smack_d_instantiate+0x5d5/0xa20
> [   77.906018][ T5418]        security_d_instantiate+0x6b/0xb0
> [   77.911736][ T5418]        d_instantiate_new+0x5e/0xe0
> [   77.917013][ T5418]   (L1) reiserfs_create+0x5ee/0x680
> [   77.922293][ T5418]        path_openat+0xf1e/0x2c10
> [   77.927308][ T5418]        do_filp_open+0x22a/0x440
> [   77.932330][ T5418]        do_sys_openat2+0x10f/0x430
> [   77.937515][ T5418]        __x64_sys_creat+0x11e/0x160
> [   77.942874][ T5418]        do_syscall_64+0x41/0xc0
> [   77.947796][ T5418]        entry_SYSCALL_64_after_hwframe+0x63/0xcd
> [   77.954200][ T5418] 
> [   77.954200][ T5418] other info that might help us debug this:
> [   77.954200][ T5418] 
> [   77.964508][ T5418]  Possible unsafe locking scenario:
> [   77.964508][ T5418] 
> [   77.972034][ T5418]   CPU0                         CPU1
> [   77.977394][ T5418]   ----                         ----
> [   77.982748][ T5418]   L1: lock(&sbi->lock);
> [   77.986726][ T5418]                                L2: lock(&type->i_mutex_dir_key#8/3);
> [   77.994618][ T5418]                                L3: lock(&sbi->lock);
> [   78.001118][ T5418]   L4: lock(&type->i_mutex_dir_key#8/3);
> 
> Thanks
> 
> Roberto
> 
> > > First of all, the lockdep warning is trivial to reproduce:
> > > 
> > > # dd if=/dev/zero of=reiserfs.img bs=1M count=100
> > > # losetup -f --show reiserfs.img
> > > /dev/loop0
> > > # mkfs.reiserfs /dev/loop0
> > > # mount /dev/loop0 /mnt/
> > > # touch file0
> > > 
> > > In the testing system, Smack is the major LSM.
> > > 
> > > Ok, so the warning here is clear:
> > > 
> > > https://syzkaller.appspot.com/x/log.txt?x=12403182280000
> > > 
> > > However, I was looking if that can really happen. From this:
> > > 
> > > [   77.746561][ T5418] -> #1 (&sbi->lock){+.+.}-{3:3}:
> > > [   77.753772][ T5418]        lock_acquire+0x23e/0x630
> > > [   77.758792][ T5418]        __mutex_lock_common+0x1d8/0x2530
> > > [   77.764504][ T5418]        mutex_lock_nested+0x1b/0x20
> > > [   77.769868][ T5418]        reiserfs_write_lock+0x70/0xc0
> > > [   77.775321][ T5418]        reiserfs_mkdir+0x321/0x870
> > > 
> > > I see that the lock is taken in reiserfs_write_lock(), while lockdep says:
> > > 
> > > [   77.710227][ T5418] but task is already holding lock:
> > > [   77.717587][ T5418] ffff88807568d090 (&sbi->lock){+.+.}-{3:3}, at: 
> > > reiserfs_write_lock_nested+0x4a/0xb0
> > > 
> > > which is in a different place, I believe here:
> > > 
> > > int reiserfs_paste_into_item(struct reiserfs_transaction_handle *th,
> > >                               /* Path to the pasted item. */
> > > [...]
> > > 
> > >          depth = reiserfs_write_unlock_nested(sb);
> > >          dquot_free_space_nodirty(inode, pasted_size);
> > >          reiserfs_write_lock_nested(sb, depth);
> > >          return retval;
> > > }
> > > 
> > > This is called by reiserfs_add_entry(), which is called by 
> > > reiserfs_create() (it is in the lockdep trace). After returning to 
> > > reiserfs_create(), d_instantiate_new() is called.
> > > 
> > > I don't know exactly, I take the part that the lock is held. But if it 
> > > is held, how d_instantiate_new() can be executed in another task?
> > > 
> > > static int reiserfs_create(struct mnt_idmap *idmap, struct inode *dir,
> > >                          struct dentry *dentry, umode_t mode, bool excl)
> > > {
> > > 
> > > [...]
> > > 
> > >          reiserfs_write_lock(dir->i_sb);
> > > 
> > >          retval = journal_begin(&th, dir->i_sb, jbegin_count);
> > > 
> > > [...]
> > > 
> > >          d_instantiate_new(dentry, inode);
> > >          retval = journal_end(&th);
> > > 
> > > out_failed:
> > >          reiserfs_write_unlock(dir->i_sb);
> > > 
> > > If the lock is held, the scenario lockdep describes cannot happen. Any 
> > > thoughts?
> > 
> > It's important to understand that the reiserfs write lock was added as a 
> > subsystem-specific replacement for the BKL.  Given that reiserfs was 
> > dying already back then, it made more sense from a time management 
> > perspective to emulate that behavior internally rather than use new 
> > locking when practically nobody cared anymore.
> > 
> > See reiserfs_write_unlock_nested and reiserfs_write_lock_nested paired 
> > throughout the code.  It drops the lock when it passes a point where 
> > it's likely to schedule, just like the BKL would have.
> > 
> > Yes, it's a mess.  Just let it die quietly.
> > 
> > -Jeff
> > 

Re: [syzbot] [reiserfs?] possible deadlock in open_xa_dir

[Roberto Sassu]

On 5/5/2023 10:51 PM, syzbot wrote:
> syzbot has bisected this issue to:
> 
> commit d82dcd9e21b77d338dc4875f3d4111f0db314a7c
> Author: Roberto Sassu <roberto.sassu@huawei.com>
> Date:   Fri Mar 31 12:32:18 2023 +0000
> 
>      reiserfs: Add security prefix to xattr name in reiserfs_security_write()
> 
> bisection log:  https://syzkaller.appspot.com/x/bisect.txt?x=14403182280000
> start commit:   3c4aa4434377 Merge tag 'ceph-for-6.4-rc1' of https://githu..
> git tree:       upstream
> final oops:     https://syzkaller.appspot.com/x/report.txt?x=16403182280000
> console output: https://syzkaller.appspot.com/x/log.txt?x=12403182280000
> kernel config:  https://syzkaller.appspot.com/x/.config?x=73a06f6ef2d5b492
> dashboard link: https://syzkaller.appspot.com/bug?extid=8fb64a61fdd96b50f3b8
> syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=12442414280000
> C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=176a7318280000
> 
> Reported-by: syzbot+8fb64a61fdd96b50f3b8@syzkaller.appspotmail.com
> Fixes: d82dcd9e21b7 ("reiserfs: Add security prefix to xattr name in reiserfs_security_write()")
> 
> For information about bisection process see: https://goo.gl/tpsmEJ#bisection

#syz test: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master

--- a/fs/reiserfs/namei.c
+++ b/fs/reiserfs/namei.c
@@ -689,7 +689,9 @@ static int reiserfs_create(struct mnt_idmap *idmap, struct inode *dir,
         reiserfs_update_inode_transaction(inode);
         reiserfs_update_inode_transaction(dir);

+       reiserfs_write_unlock(dir->i_sb);
         d_instantiate_new(dentry, inode);
+       reiserfs_write_lock(dir->i_sb);
         retval = journal_end(&th);

  out_failed:
@@ -773,7 +775,9 @@ static int reiserfs_mknod(struct mnt_idmap *idmap, struct inode *dir,
                 goto out_failed;
         }

+       reiserfs_write_unlock(dir->i_sb);
         d_instantiate_new(dentry, inode);
+       reiserfs_write_lock(dir->i_sb);
         retval = journal_end(&th);

  out_failed:
@@ -874,7 +878,9 @@ static int reiserfs_mkdir(struct mnt_idmap *idmap, struct inode *dir,
         /* the above add_entry did not update dir's stat data */
         reiserfs_update_sd(&th, dir);

+       reiserfs_write_unlock(dir->i_sb);
         d_instantiate_new(dentry, inode);
+       reiserfs_write_lock(dir->i_sb);
         retval = journal_end(&th);
  out_failed:
         reiserfs_write_unlock(dir->i_sb);
@@ -1191,7 +1197,9 @@ static int reiserfs_symlink(struct mnt_idmap *idmap,
                 goto out_failed;
         }

+       reiserfs_write_unlock(parent_dir->i_sb);
         d_instantiate_new(dentry, inode);
+       reiserfs_write_lock(parent_dir->i_sb);
         retval = journal_end(&th);
  out_failed:
         reiserfs_write_unlock(parent_dir->i_sb);

Re: [syzbot] [reiserfs?] possible deadlock in open_xa_dir

[syzbot]

Hello,

syzbot tried to test the proposed patch but the build/boot failed:

failed to apply patch:
checking file fs/reiserfs/namei.c
patch: **** unexpected end of file in patch



Tested on:

commit:         929ed21d Merge tag '6.4-rc4-smb3-client-fixes' of git:..
git tree:       upstream
dashboard link: https://syzkaller.appspot.com/bug?extid=8fb64a61fdd96b50f3b8
compiler:       
patch:          https://syzkaller.appspot.com/x/patch.diff?x=10efc079280000

Re: [syzbot] [reiserfs?] possible deadlock in open_xa_dir

[Roberto Sassu]

[-- Attachment #1: Type: text/plain, Size: 1285 bytes --]

On 5/5/2023 10:51 PM, syzbot wrote:
> syzbot has bisected this issue to:
> 
> commit d82dcd9e21b77d338dc4875f3d4111f0db314a7c
> Author: Roberto Sassu <roberto.sassu@huawei.com>
> Date:   Fri Mar 31 12:32:18 2023 +0000
> 
>      reiserfs: Add security prefix to xattr name in reiserfs_security_write()
> 
> bisection log:  https://syzkaller.appspot.com/x/bisect.txt?x=14403182280000
> start commit:   3c4aa4434377 Merge tag 'ceph-for-6.4-rc1' of https://githu..
> git tree:       upstream
> final oops:     https://syzkaller.appspot.com/x/report.txt?x=16403182280000
> console output: https://syzkaller.appspot.com/x/log.txt?x=12403182280000
> kernel config:  https://syzkaller.appspot.com/x/.config?x=73a06f6ef2d5b492
> dashboard link: https://syzkaller.appspot.com/bug?extid=8fb64a61fdd96b50f3b8
> syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=12442414280000
> C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=176a7318280000
> 
> Reported-by: syzbot+8fb64a61fdd96b50f3b8@syzkaller.appspotmail.com
> Fixes: d82dcd9e21b7 ("reiserfs: Add security prefix to xattr name in reiserfs_security_write()")
> 
> For information about bisection process see: https://goo.gl/tpsmEJ#bisection

#syz test: git://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git master

[-- Attachment #2: 0001-reiserfs-Move-d_instantiate_new-out-of-the-write-loc.patch --]
[-- Type: text/plain, Size: 2724 bytes --]

From cf5445afc351bbc55a0080f1bc408ff496aeb879 Mon Sep 17 00:00:00 2001
From: Roberto Sassu <roberto.sassu@huawei.com>
Date: Thu, 1 Jun 2023 20:36:37 +0200
Subject: [RFC][PATCH] reiserfs: Move d_instantiate_new() out of the write lock

Commit 4c05141df57f ("reiserfs: locking, push write lock out of xattr
code") moved xattr operations outside the write lock. The problem is that
not all xattr operations are outside that lock.  For example, the write
lock is not released when d_instantiate_new() is called. At that time,
active LSMs likely fetch the content from their xattrs.

Mixing the two cases (xattr operations without and with a write lock)
could cause a deadlock. For example, a deadlock could happen due to the
following circular dependencies:

write lock (task A) -> inode lock (task B) ->write lock (task B)
-> inode lock (task A)

Make sure that all xattr operations are outside the write lock, by
wrapping all d_instantiate_new() calls with reiserfs_write_unlock() and
reiserfs_write_lock().

Fixes: d82dcd9e21b7 ("reiserfs: Add security prefix to xattr name in reiserfs_security_write()")
Reported-by: syzbot+8fb64a61fdd96b50f3b8@syzkaller.appspotmail.com
Signed-off-by: Roberto Sassu <roberto.sassu@huawei.com>
---
 fs/reiserfs/namei.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/fs/reiserfs/namei.c b/fs/reiserfs/namei.c
index 52240cc891c..3508bf1a75e 100644
--- a/fs/reiserfs/namei.c
+++ b/fs/reiserfs/namei.c
@@ -689,7 +689,9 @@ static int reiserfs_create(struct mnt_idmap *idmap, struct inode *dir,
 	reiserfs_update_inode_transaction(inode);
 	reiserfs_update_inode_transaction(dir);
 
+	reiserfs_write_unlock(dir->i_sb);
 	d_instantiate_new(dentry, inode);
+	reiserfs_write_lock(dir->i_sb);
 	retval = journal_end(&th);
 
 out_failed:
@@ -773,7 +775,9 @@ static int reiserfs_mknod(struct mnt_idmap *idmap, struct inode *dir,
 		goto out_failed;
 	}
 
+	reiserfs_write_unlock(dir->i_sb);
 	d_instantiate_new(dentry, inode);
+	reiserfs_write_lock(dir->i_sb);
 	retval = journal_end(&th);
 
 out_failed:
@@ -874,7 +878,9 @@ static int reiserfs_mkdir(struct mnt_idmap *idmap, struct inode *dir,
 	/* the above add_entry did not update dir's stat data */
 	reiserfs_update_sd(&th, dir);
 
+	reiserfs_write_unlock(dir->i_sb);
 	d_instantiate_new(dentry, inode);
+	reiserfs_write_lock(dir->i_sb);
 	retval = journal_end(&th);
 out_failed:
 	reiserfs_write_unlock(dir->i_sb);
@@ -1191,7 +1197,9 @@ static int reiserfs_symlink(struct mnt_idmap *idmap,
 		goto out_failed;
 	}
 
+	reiserfs_write_unlock(parent_dir->i_sb);
 	d_instantiate_new(dentry, inode);
+	reiserfs_write_lock(parent_dir->i_sb);
 	retval = journal_end(&th);
 out_failed:
 	reiserfs_write_unlock(parent_dir->i_sb);
-- 
2.25.1

Re: [syzbot] [reiserfs?] possible deadlock in open_xa_dir

[syzbot]

Hello,

syzbot has tested the proposed patch and the reproducer did not trigger any issue:

Reported-and-tested-by: syzbot+8fb64a61fdd96b50f3b8@syzkaller.appspotmail.com

Tested on:

commit:         929ed21d Merge tag '6.4-rc4-smb3-client-fixes' of git:..
git tree:       upstream
console output: https://syzkaller.appspot.com/x/log.txt?x=16c69fed280000
kernel config:  https://syzkaller.appspot.com/x/.config?x=162cf2103e4a7453
dashboard link: https://syzkaller.appspot.com/bug?extid=8fb64a61fdd96b50f3b8
compiler:       Debian clang version 15.0.7, GNU ld (GNU Binutils for Debian) 2.35.2
patch:          https://syzkaller.appspot.com/x/patch.diff?x=1159c149280000

Note: testing is done by a robot and is best-effort only.

Re: [syzbot] [reiserfs?] possible deadlock in open_xa_dir

[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=11c28556180000
start commit:   5eff55d725a4 Merge tag 'platform-drivers-x86-v6.7-7' of gi..
git tree:       upstream
kernel config:  https://syzkaller.appspot.com/x/.config?x=655f8abe9fe69b3b
dashboard link: https://syzkaller.appspot.com/bug?extid=8fb64a61fdd96b50f3b8
syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=12d80b99e80000
C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=148cccdee80000

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