linux-kernel - Re: [PATCH] kcsan, debugfs: Move debugfs file creation out of early init

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] [thread-next>] [day] [month] [year] [list]

Date:   Wed, 3 Mar 2021 11:23:54 +0100
From:   Greg KH <gregkh@...uxfoundation.org>
To:     Marco Elver <elver@...gle.com>
Cc:     rafael@...nel.org, "Paul E. McKenney" <paulmck@...nel.org>,
        Dmitry Vyukov <dvyukov@...gle.com>,
        Alexander Potapenko <glider@...gle.com>,
        Andrey Konovalov <andreyknvl@...gle.com>,
        kasan-dev <kasan-dev@...glegroups.com>,
        LKML <linux-kernel@...r.kernel.org>,
        stable <stable@...r.kernel.org>
Subject: Re: [PATCH] kcsan, debugfs: Move debugfs file creation out of early
 init

On Wed, Mar 03, 2021 at 11:18:06AM +0100, Marco Elver wrote:
> On Wed, 3 Mar 2021 at 10:57, Greg KH <gregkh@...uxfoundation.org> wrote:
> >
> > On Wed, Mar 03, 2021 at 10:38:45AM +0100, Marco Elver wrote:
> > > Commit 56348560d495 ("debugfs: do not attempt to create a new file
> > > before the filesystem is initalized") forbids creating new debugfs files
> > > until debugfs is fully initialized. This breaks KCSAN's debugfs file
> > > creation, which happened at the end of __init().
> >
> > How did it "break" it?  The files shouldn't have actually been created,
> > right?
> 
> Right, with 56348560d495 the debugfs file isn't created anymore, which
> is the problem. Before 56348560d495 the file exists (syzbot wants the
> file to exist.)
> 
> > > There is no reason to create the debugfs file during early
> > > initialization. Therefore, move it into a late_initcall() callback.
> > >
> > > Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org>
> > > Cc: "Rafael J. Wysocki" <rafael@...nel.org>
> > > Cc: stable <stable@...r.kernel.org>
> > > Fixes: 56348560d495 ("debugfs: do not attempt to create a new file before the filesystem is initalized")
> > > Signed-off-by: Marco Elver <elver@...gle.com>
> > > ---
> > > I've marked this for 'stable', since 56348560d495 is also intended for
> > > stable, and would subsequently break KCSAN in all stable kernels where
> > > KCSAN is available (since 5.8).
> >
> > No objection from me, just odd that this actually fixes anything :)
> 
> 56348560d495 causes the file to just not be created if we try to
> create at the end of __init(). Having it created as late as
> late_initcall() gets us the file back.
> 
> When you say "fixes anything", should the file be created even though
> it's at the end of __init()? Perhaps I misunderstood what 56348560d495
> changes, but I verified it to be the problem by reverting (upon which
> the file exists as expected).

All my change did is explicitly not allow you to create a file if
debugfs had not been initialized.  If you tried to do that before, you
should have gotten an error from the vfs layer that the file was not
created, as otherwise how would it have succeeded?

I just moved the check up higher in the "stack" to the debugfs code, and
not relied on the vfs layer to do a lot of work only to reject things
later on.

So there "should" not have been any functional change with this patch.
If there was, then something is really odd as how can the vfs layer
create a file for a filesystem _before_ that filesystem has been
registered with the vfs layer?

thanks,

greg k-h