linux-kernel - Re: [PATCH v3 1/2] fs/super: function to prevent super re-use

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [day] [month] [year] [list]

Message-ID: <CAONX=-dQuiG26gCp7FVfZk83au=RXXcnTWJO8+Ygep0iVqvnxA@mail.gmail.com>
Date:   Wed, 1 Jun 2022 11:11:55 +1000
From:   Daniil Lunev <dlunev@...omium.org>
To:     Christoph Hellwig <hch@...radead.org>
Cc:     linux-fsdevel@...r.kernel.org, miklos@...redi.hu,
        viro@...iv.linux.org.uk, tytso@....edu,
        fuse-devel@...ts.sourceforge.net, linux-kernel@...r.kernel.org
Subject: Re: [PATCH v3 1/2] fs/super: function to prevent super re-use

Thank you for your comments. Uploaded v4 with requested changes.
--Daniil

On Tue, May 31, 2022 at 5:24 PM Christoph Hellwig <hch@...radead.org> wrote:
>
> On Mon, May 30, 2022 at 11:39:57AM +1000, Daniil Lunev wrote:
> > +void retire_super(struct super_block *sb)
> > +{
> > +     down_write(&sb->s_umount);
> > +     if (sb->s_bdi != &noop_backing_dev_info) {
> > +             if (sb->s_iflags & SB_I_PERSB_BDI && !(sb->s_iflags & SB_I_RETIRED))
>
> SB_I_PERSB_BDI can't be set for noop_backing_dev_info, so that check
> should not be needed.  Which also conveniently fixes the overly long
> line.
>
> Also this should clear SB_I_PERSB_BDI as the only place that checks
> it is the unregistration.
>
> >       spin_lock(&sb_lock);
> > -     /* should be initialized for __put_super_and_need_restart() */
>
> This is a completely unrelated change.  While the function is gone
> it might be worth to check what it got renamed to or folded in, or
> if the initialization is still needed.  But all that is for a separate
> patch.
>
> >       up_write(&sb->s_umount);
> >       if (sb->s_bdi != &noop_backing_dev_info) {
> > -             if (sb->s_iflags & SB_I_PERSB_BDI)
> > +             /* retire should have already unregistered bdi */
> > +             if (sb->s_iflags & SB_I_PERSB_BDI && !(sb->s_iflags & SB_I_RETIRED))
> >                       bdi_unregister(sb->s_bdi);
> >               bdi_put(sb->s_bdi);
>
> And once SB_I_PERSB_BDI is dropped when retiring we don't need this
> change.