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

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