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

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

[syzbot]

syzbot has found a reproducer for the following issue on:

HEAD commit:    e40939bbfc68 Merge branch 'for-next/core' into for-kernelci
git tree:       git://git.kernel.org/pub/scm/linux/kernel/git/arm64/linux.git for-kernelci
console output: https://syzkaller.appspot.com/x/log.txt?x=1070f4bca80000
kernel config:  https://syzkaller.appspot.com/x/.config?x=c84f463eb74eab24
dashboard link: https://syzkaller.appspot.com/bug?extid=c319bb5b1014113a92cf
compiler:       Debian clang version 15.0.7, GNU ld (GNU Binutils for Debian) 2.35.2
userspace arch: arm64
syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=112c37e2a80000
C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=12e4c2daa80000

Downloadable assets:
disk image: https://storage.googleapis.com/syzbot-assets/257596b75aaf/disk-e40939bb.raw.xz
vmlinux: https://storage.googleapis.com/syzbot-assets/9c75b8d61081/vmlinux-e40939bb.xz
kernel image: https://storage.googleapis.com/syzbot-assets/8f0233129f4f/Image-e40939bb.gz.xz
mounted in repro: https://storage.googleapis.com/syzbot-assets/05e314af739c/mount_0.gz

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

======================================================
WARNING: possible circular locking dependency detected
6.4.0-rc7-syzkaller-ge40939bbfc68 #0 Not tainted
------------------------------------------------------
syz-executor365/5984 is trying to acquire lock:
ffff0000db0d17d8 (&mm->mmap_lock
){++++}-{3:3}, at: __might_fault+0x9c/0x124 mm/memory.c:5731

but task is already holding lock:
ffff0000c2367090 (&sbi->lock){+.+.}-{3:3}, at: reiserfs_write_lock+0x7c/0xe8 fs/reiserfs/lock.c:27

which lock already depends on the new lock.


the existing dependency chain (in reverse order) is:

-> #1 (&sbi->lock){+.+.}-{3:3}:
       __mutex_lock_common+0x190/0x21a0 kernel/locking/mutex.c:603
       __mutex_lock kernel/locking/mutex.c:747 [inline]
       mutex_lock_nested+0x2c/0x38 kernel/locking/mutex.c:799
       reiserfs_write_lock+0x7c/0xe8 fs/reiserfs/lock.c:27
       reiserfs_dirty_inode+0xe4/0x204 fs/reiserfs/super.c:704
       __mark_inode_dirty+0x2b0/0x10f4 fs/fs-writeback.c:2424
       generic_update_time fs/inode.c:1859 [inline]
       inode_update_time fs/inode.c:1872 [inline]
       touch_atime+0x5d8/0x8d4 fs/inode.c:1944
       file_accessed include/linux/fs.h:2198 [inline]
       generic_file_mmap+0xb0/0x11c mm/filemap.c:3606
       call_mmap include/linux/fs.h:1873 [inline]
       mmap_region+0xc00/0x1aa4 mm/mmap.c:2649
       do_mmap+0xa00/0x1108 mm/mmap.c:1394
       vm_mmap_pgoff+0x198/0x3b8 mm/util.c:543
       ksys_mmap_pgoff+0x3c8/0x5b0 mm/mmap.c:1440
       __do_sys_mmap arch/arm64/kernel/sys.c:28 [inline]
       __se_sys_mmap arch/arm64/kernel/sys.c:21 [inline]
       __arm64_sys_mmap+0xf8/0x110 arch/arm64/kernel/sys.c:21
       __invoke_syscall arch/arm64/kernel/syscall.c:38 [inline]
       invoke_syscall+0x98/0x2c0 arch/arm64/kernel/syscall.c:52
       el0_svc_common+0x138/0x244 arch/arm64/kernel/syscall.c:142
       do_el0_svc+0x64/0x198 arch/arm64/kernel/syscall.c:191
       el0_svc+0x4c/0x160 arch/arm64/kernel/entry-common.c:647
       el0t_64_sync_handler+0x84/0xfc arch/arm64/kernel/entry-common.c:665
       el0t_64_sync+0x190/0x194 arch/arm64/kernel/entry.S:591

-> #0 (&mm->mmap_lock){++++}-{3:3}:
       check_prev_add kernel/locking/lockdep.c:3113 [inline]
       check_prevs_add kernel/locking/lockdep.c:3232 [inline]
       validate_chain kernel/locking/lockdep.c:3847 [inline]
       __lock_acquire+0x3308/0x7604 kernel/locking/lockdep.c:5088
       lock_acquire+0x23c/0x71c kernel/locking/lockdep.c:5705
       __might_fault+0xc4/0x124 mm/memory.c:5732
       reiserfs_ioctl+0x10c/0x454 fs/reiserfs/ioctl.c:96
       vfs_ioctl fs/ioctl.c:51 [inline]
       __do_sys_ioctl fs/ioctl.c:870 [inline]
       __se_sys_ioctl fs/ioctl.c:856 [inline]
       __arm64_sys_ioctl+0x14c/0x1c8 fs/ioctl.c:856
       __invoke_syscall arch/arm64/kernel/syscall.c:38 [inline]
       invoke_syscall+0x98/0x2c0 arch/arm64/kernel/syscall.c:52
       el0_svc_common+0x138/0x244 arch/arm64/kernel/syscall.c:142
       do_el0_svc+0x64/0x198 arch/arm64/kernel/syscall.c:191
       el0_svc+0x4c/0x160 arch/arm64/kernel/entry-common.c:647
       el0t_64_sync_handler+0x84/0xfc arch/arm64/kernel/entry-common.c:665
       el0t_64_sync+0x190/0x194 arch/arm64/kernel/entry.S:591

other info that might help us debug this:

 Possible unsafe locking scenario:

       CPU0                    CPU1
       ----                    ----
  lock(&sbi->lock);
                               lock(&mm->mmap_lock);
                               lock(&sbi->lock);
  rlock(&mm->mmap_lock);

 *** DEADLOCK ***

1 lock held by syz-executor365/5984:
 #0: ffff0000c2367090 (&sbi->lock){+.+.}-{3:3}, at: reiserfs_write_lock+0x7c/0xe8 fs/reiserfs/lock.c:27

stack backtrace:
CPU: 1 PID: 5984 Comm: syz-executor365 Not tainted 6.4.0-rc7-syzkaller-ge40939bbfc68 #0
Hardware name: Google Google Compute Engine/Google Compute Engine, BIOS Google 05/27/2023
Call trace:
 dump_backtrace+0x1b8/0x1e4 arch/arm64/kernel/stacktrace.c:233
 show_stack+0x2c/0x44 arch/arm64/kernel/stacktrace.c:240
 __dump_stack lib/dump_stack.c:88 [inline]
 dump_stack_lvl+0xd0/0x124 lib/dump_stack.c:106
 dump_stack+0x1c/0x28 lib/dump_stack.c:113
 print_circular_bug+0x150/0x1b8 kernel/locking/lockdep.c:2066
 check_noncircular+0x2cc/0x378 kernel/locking/lockdep.c:2188
 check_prev_add kernel/locking/lockdep.c:3113 [inline]
 check_prevs_add kernel/locking/lockdep.c:3232 [inline]
 validate_chain kernel/locking/lockdep.c:3847 [inline]
 __lock_acquire+0x3308/0x7604 kernel/locking/lockdep.c:5088
 lock_acquire+0x23c/0x71c kernel/locking/lockdep.c:5705
 __might_fault+0xc4/0x124 mm/memory.c:5732
 reiserfs_ioctl+0x10c/0x454 fs/reiserfs/ioctl.c:96
 vfs_ioctl fs/ioctl.c:51 [inline]
 __do_sys_ioctl fs/ioctl.c:870 [inline]
 __se_sys_ioctl fs/ioctl.c:856 [inline]
 __arm64_sys_ioctl+0x14c/0x1c8 fs/ioctl.c:856
 __invoke_syscall arch/arm64/kernel/syscall.c:38 [inline]
 invoke_syscall+0x98/0x2c0 arch/arm64/kernel/syscall.c:52
 el0_svc_common+0x138/0x244 arch/arm64/kernel/syscall.c:142
 do_el0_svc+0x64/0x198 arch/arm64/kernel/syscall.c:191
 el0_svc+0x4c/0x160 arch/arm64/kernel/entry-common.c:647
 el0t_64_sync_handler+0x84/0xfc arch/arm64/kernel/entry-common.c:665
 el0t_64_sync+0x190/0x194 arch/arm64/kernel/entry.S:591


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

[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=14d0b787680000
start commit:   90b0c2b2edd1 Merge tag 'pinctrl-v6.7-1' of git://git.kerne..
git tree:       upstream
final oops:     https://syzkaller.appspot.com/x/report.txt?x=16d0b787680000
console output: https://syzkaller.appspot.com/x/log.txt?x=12d0b787680000
kernel config:  https://syzkaller.appspot.com/x/.config?x=93ac5233c138249e
dashboard link: https://syzkaller.appspot.com/bug?extid=c319bb5b1014113a92cf
syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=113f3717680000
C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=154985ef680000

Reported-by: syzbot+c319bb5b1014113a92cf@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 reiserfs_dirty_inode

[Paul Moore]

On Mon, Nov 6, 2023 at 3:34 AM syzbot
<syzbot+c319bb5b1014113a92cf@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=14d0b787680000
> start commit:   90b0c2b2edd1 Merge tag 'pinctrl-v6.7-1' of git://git.kerne..
> git tree:       upstream
> final oops:     https://syzkaller.appspot.com/x/report.txt?x=16d0b787680000
> console output: https://syzkaller.appspot.com/x/log.txt?x=12d0b787680000
> kernel config:  https://syzkaller.appspot.com/x/.config?x=93ac5233c138249e
> dashboard link: https://syzkaller.appspot.com/bug?extid=c319bb5b1014113a92cf
> syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=113f3717680000
> C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=154985ef680000
>
> Reported-by: syzbot+c319bb5b1014113a92cf@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

Hi Roberto,

I know you were looking at this over the summer[1], did you ever find
a resolution to this?  If not, what do you think of just dropping
security xattr support on reiserfs?  Normally that wouldn't be
something we could consider, but given the likelihood that this hadn't
been working in *years* (if ever), and reiserfs is deprecated, I think
this is a viable option if there isn't an obvious fix.

[1] https://lore.kernel.org/linux-security-module/CAHC9VhTM0a7jnhxpCyonepcfWbnG-OJbbLpjQi68gL2GVnKSRg@mail.gmail.com/

-- 
paul-moore.com

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

[Roberto Sassu]

On Mon, 2023-11-06 at 17:53 -0500, Paul Moore wrote:
> On Mon, Nov 6, 2023 at 3:34 AM syzbot
> <syzbot+c319bb5b1014113a92cf@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=14d0b787680000
> > start commit:   90b0c2b2edd1 Merge tag 'pinctrl-v6.7-1' of git://git.kerne..
> > git tree:       upstream
> > final oops:     https://syzkaller.appspot.com/x/report.txt?x=16d0b787680000
> > console output: https://syzkaller.appspot.com/x/log.txt?x=12d0b787680000
> > kernel config:  https://syzkaller.appspot.com/x/.config?x=93ac5233c138249e
> > dashboard link: https://syzkaller.appspot.com/bug?extid=c319bb5b1014113a92cf
> > syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=113f3717680000
> > C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=154985ef680000
> > 
> > Reported-by: syzbot+c319bb5b1014113a92cf@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
> 
> Hi Roberto,
> 
> I know you were looking at this over the summer[1], did you ever find
> a resolution to this?  If not, what do you think of just dropping
> security xattr support on reiserfs?  Normally that wouldn't be
> something we could consider, but given the likelihood that this hadn't
> been working in *years* (if ever), and reiserfs is deprecated, I think
> this is a viable option if there isn't an obvious fix.
> 
> [1] https://lore.kernel.org/linux-security-module/CAHC9VhTM0a7jnhxpCyonepcfWbnG-OJbbLpjQi68gL2GVnKSRg@mail.gmail.com/

Hi Paul

at the time, I did some investigation and came with a patch that
(likely) solves some of the problems:

https://lore.kernel.org/linux-fsdevel/4aa799a0b87d4e2ecf3fa74079402074dc42b3c5.camel@huaweicloud.com/#t

I did a more advanced patch (to be validated), trying to fix the root
cause:

https://lore.kernel.org/linux-fsdevel/ffde7908-be73-cc56-2646-72f4f94cb51b@huaweicloud.com/

However, Jeff Mahoney (that did a lot of work in this area) suggested
that maybe we should not try invasive changes, as anyway reiserfs will
be removed from the kernel in 2025.

It wouldn't be a problem to move the first patch forward.

Roberto

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

[Paul Moore]

On Tue, Nov 7, 2023 at 6:03 AM Roberto Sassu
<roberto.sassu@huaweicloud.com> wrote:
> On Mon, 2023-11-06 at 17:53 -0500, Paul Moore wrote:
> > Hi Roberto,
> >
> > I know you were looking at this over the summer[1], did you ever find
> > a resolution to this?  If not, what do you think of just dropping
> > security xattr support on reiserfs?  Normally that wouldn't be
> > something we could consider, but given the likelihood that this hadn't
> > been working in *years* (if ever), and reiserfs is deprecated, I think
> > this is a viable option if there isn't an obvious fix.
> >
> > [1] https://lore.kernel.org/linux-security-module/CAHC9VhTM0a7jnhxpCyonepcfWbnG-OJbbLpjQi68gL2GVnKSRg@mail.gmail.com/
>
> Hi Paul
>
> at the time, I did some investigation and came with a patch that
> (likely) solves some of the problems:
>
> https://lore.kernel.org/linux-fsdevel/4aa799a0b87d4e2ecf3fa74079402074dc42b3c5.camel@huaweicloud.com/#t

Ah, thanks for the link, it looks like that was swallowed by my inbox.
In general if you feel it is worth adding my email to a patch, you
should probably also CC the LSM list.  If nothing else there is a
patchwork watching the LSM list that I use to make sure I don't
miss/forget about patches.

> I did a more advanced patch (to be validated), trying to fix the root
> cause:
>
> https://lore.kernel.org/linux-fsdevel/ffde7908-be73-cc56-2646-72f4f94cb51b@huaweicloud.com/
>
> However, Jeff Mahoney (that did a lot of work in this area) suggested
> that maybe we should not try invasive changes, as anyway reiserfs will
> be removed from the kernel in 2025.

I tend to agree with Jeff, which is one of the reasons I was
suggesting simply removing LSM xattr support from reiserfs, although
depending on what that involves it might be a big enough change that
we are better off simply leaving it broken.  I think we need to see
what that patch would look like first.

> It wouldn't be a problem to move the first patch forward.

I worry that the first patch you mentioned above doesn't really solve
anything, it only makes it the responsibility of the user to choose
either A) a broken system where LSM xattrs don't work or B) a system
that will likely deadlock/panic.  I think I would rather revert the
original commit and just leave the LSM xattrs broken than ask a user
to make that choice.

-- 
paul-moore.com

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

[Roberto Sassu]

On Tue, 2023-11-07 at 17:26 -0500, Paul Moore wrote:
> On Tue, Nov 7, 2023 at 6:03 AM Roberto Sassu
> <roberto.sassu@huaweicloud.com> wrote:
> > On Mon, 2023-11-06 at 17:53 -0500, Paul Moore wrote:
> > > Hi Roberto,
> > > 
> > > I know you were looking at this over the summer[1], did you ever find
> > > a resolution to this?  If not, what do you think of just dropping
> > > security xattr support on reiserfs?  Normally that wouldn't be
> > > something we could consider, but given the likelihood that this hadn't
> > > been working in *years* (if ever), and reiserfs is deprecated, I think
> > > this is a viable option if there isn't an obvious fix.
> > > 
> > > [1] https://lore.kernel.org/linux-security-module/CAHC9VhTM0a7jnhxpCyonepcfWbnG-OJbbLpjQi68gL2GVnKSRg@mail.gmail.com/
> > 
> > Hi Paul
> > 
> > at the time, I did some investigation and came with a patch that
> > (likely) solves some of the problems:
> > 
> > https://lore.kernel.org/linux-fsdevel/4aa799a0b87d4e2ecf3fa74079402074dc42b3c5.camel@huaweicloud.com/#t
> 
> Ah, thanks for the link, it looks like that was swallowed by my inbox.
> In general if you feel it is worth adding my email to a patch, you
> should probably also CC the LSM list.  If nothing else there is a
> patchwork watching the LSM list that I use to make sure I don't
> miss/forget about patches.
> 
> > I did a more advanced patch (to be validated), trying to fix the root
> > cause:
> > 
> > https://lore.kernel.org/linux-fsdevel/ffde7908-be73-cc56-2646-72f4f94cb51b@huaweicloud.com/
> > 
> > However, Jeff Mahoney (that did a lot of work in this area) suggested
> > that maybe we should not try invasive changes, as anyway reiserfs will
> > be removed from the kernel in 2025.
> 
> I tend to agree with Jeff, which is one of the reasons I was
> suggesting simply removing LSM xattr support from reiserfs, although
> depending on what that involves it might be a big enough change that
> we are better off simply leaving it broken.  I think we need to see
> what that patch would look like first.
> 
> > It wouldn't be a problem to move the first patch forward.
> 
> I worry that the first patch you mentioned above doesn't really solve
> anything, it only makes it the responsibility of the user to choose
> either A) a broken system where LSM xattrs don't work or B) a system
> that will likely deadlock/panic.  I think I would rather revert the
> original commit and just leave the LSM xattrs broken than ask a user
> to make that choice.

Ok, that would be fine for me.

Thanks

Roberto

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

[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=11750da6180000
start commit:   90b0c2b2edd1 Merge tag 'pinctrl-v6.7-1' of git://git.kerne..
git tree:       upstream
kernel config:  https://syzkaller.appspot.com/x/.config?x=93ac5233c138249e
dashboard link: https://syzkaller.appspot.com/bug?extid=c319bb5b1014113a92cf
syz repro:      https://syzkaller.appspot.com/x/repro.syz?x=113f3717680000
C reproducer:   https://syzkaller.appspot.com/x/repro.c?x=154985ef680000

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] [reiserfs?] possible deadlock in reiserfs_dirty_inode

[Tetsuo Handa]

#syz fix: fs: Block writes to mounted block devices