Message-ID: <20160906230431.GT3296@wotan.suse.de>
Date:   Wed, 7 Sep 2016 01:04:31 +0200
From:   "Luis R. Rodriguez" <mcgrof@...nel.org>
To:     Linus Torvalds <torvalds@...ux-foundation.org>
Cc:     Bjorn Andersson <bjorn.andersson@...aro.org>,
        "Luis R. Rodriguez" <mcgrof@...nel.org>,
        Daniel Vetter <daniel.vetter@...ll.ch>,
        Mimi Zohar <zohar@...ux.vnet.ibm.com>,
        Felix Fietkau <nbd@....name>,
        David Woodhouse <dwmw2@...radead.org>,
        Roman Pen <r.peniaev@...il.com>,
        Ming Lei <ming.lei@...onical.com>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Michal Marek <mmarek@...e.com>,
        Greg KH <gregkh@...uxfoundation.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Vikram Mulukutla <markivx@...eaurora.org>,
        Stephen Boyd <stephen.boyd@...aro.org>,
        Mark Brown <broonie@...nel.org>, Takashi Iwai <tiwai@...e.de>,
        Johannes Berg <johannes@...solutions.net>,
        Christian Lamparter <chunkeey@...glemail.com>,
        Hauke Mehrtens <hauke@...ke-m.de>,
        Josh Boyer <jwboyer@...oraproject.org>,
        Dmitry Torokhov <dmitry.torokhov@...il.com>,
        Jiri Slaby <jslaby@...e.com>,
        Andy Lutomirski <luto@...capital.net>,
        Wu Fengguang <fengguang.wu@...el.com>,
        Richard Purdie <rpurdie@...ys.net>,
        Jeff Mahoney <jeffm@...e.com>,
        Jacek Anaszewski <j.anaszewski@...sung.com>,
        Abhay_Salunke@...l.com, Julia Lawall <Julia.Lawall@...6.fr>,
        Gilles.Muller@...6.fr, nicolas.palix@...g.fr,
        Tom Gundersen <teg@...m.no>, Kay Sievers <kay@...y.org>,
        David Howells <dhowells@...hat.com>,
        Alessandro Rubini <rubini@...dd.com>,
        Kevin Cernekee <cernekee@...il.com>,
        Kees Cook <keescook@...omium.org>,
        Jonathan Corbet <corbet@....net>,
        Thierry Martinez <martinez@...p.org>,
        linux-serial <linux-serial@...r.kernel.org>,
        "open list:DOCUMENTATION" <linux-doc@...r.kernel.org>,
        linuxppc-dev <linuxppc-dev@...ts.ozlabs.org>
Subject: Re: [RFC] fs: add userspace critical mounts event support

On Tue, Sep 06, 2016 at 02:50:51PM -0700, Linus Torvalds wrote:
> On Tue, Sep 6, 2016 at 2:11 PM, Bjorn Andersson
> <bjorn.andersson@...aro.org> wrote:
> > On Tue 06 Sep 11:32 PDT 2016, Linus Torvalds wrote:
> >
> >> On Tue, Sep 6, 2016 at 10:46 AM, Bjorn Andersson
> >> Nobody has actually answered the "why don't we just tie the firmware
> >> and module together" question.
> >
> > The answer to this depends on the details of the suggestion; but
> > generally there's a much stronger bond between the kernel and the driver
> > than between the driver and the firmware in my cases.
> 
> I call BS.
> 
> Let me be very clear. I'm not applying that shit-for-brains stupid
> patch, and will not be pulling it unless somebody tricks me into it.

Great thanks. Please note that the only reason I proposed this was to
get the ball rolling in terms of finding a solution to the problem for why
some folks claim they *need* the firmware usermode helper. Granted, upstream
we only have 2 explicit users left, I'm told some out-of-tree users still
need and use the usermode helper.

They claim that without it there is the race between /lib/firmware being ready
and driver asking for the firmware. I was told there were quite a bit
of out-of-tree hacks to address this without using the usermode helper,
the goal of this patch was to create the discussion needed to a proper
resolution to this.

Given that upon inspection -- I've determined that even if you stuff the
firmware into initramfs you may still run into the race (the commit log of this
patch explains that) and that using initramfs was the alternative solution we
expected folks to use -- the only current deterministic sure way a driver can
depend on a firmware and avoid the race is to use CONFIG_EXTRA_FIRMWARE. This
is an issue.

> Because all these arguments make no sense at all.
> 
> If the driver doesn't work without the firmware, then anybody who
> distributes the driver binary without a firmware is just
> *fundamentally* doing something insane.

Some companies only redistribute firmware binaries to specific entities due to
avoid expanding to whom a patent grant is given to. That is, not every company
writing drivers or pushing out binary drivers is willing to dish out the
firmware as per the terms in linux-firmware.

> You may do it for *development* purposes, but doing so for actual *use* would
> be entirely pointless.

To be fair we haven't been explicit about our requirements for firmware_class
and expectations about what we expect for firmware for driver in Linux. This
has all been rather loose.

Furthermore the race issues we have found in firmware_class have only come about
through introspection, and I've been slowly documenting all this tribal knowledge.

> See my point? If a distribution is distributing the driver without the
> firmware, then what the hell is the point of such a thing?

Agreed.

> But even if you decide to do that for some odd reason, the patch is
> still just stupid. Instead of adding some crazy infrastructure for
> "now I've mounted everything", you could equally well just
> 
>  (a) make the driver fail the module load if it cannot find a firmware binary

Not sure if its clear but:

0) this is not just about firmware anymore
1) there is a race between using kernel_read_file_from_path() and
   having the filesystem that should be present on be ready;
2) Only *userspace* can know for sure what the real valid filesystem for
   files read from kernel_read_file_from_path() can be ready, so only userspace
   can tell the kernel if a read from kernel_read_file_from_path() is
   at a certain point in time valid.

>  (b) after user space has mounted everything, just do "insmod -a"
> again (or insmod just that driver).

I'm happy to document this as the resolution... I have a feeling some folks
will not like it. We also have built-in drivers to consider, what do we
advise for that? Keep in mind only CONFIG_EXTRA_FIRMWARE is deterministically
safe.

> See? The point is, this "generic" hacky interface is just stupid. It's
> not adding any value. If you add user space "I'm ready now" points
> anyway, you might as well make those points do the right thing and
> just load the module that is now loadable.

This is not about firmware anymore though, and we need to address built-in.

> We could mark such "late loading" modules explicitly if people want
> to, so that you can automate the whole thing about delaying the
> loading in user space.

Now *that* could actually help, for instance add another late
init call which would be called after initramfs and friends -- perhaps
way after prepare_namespace() -- by then we would ensure userspace has
all critical fs mounted. The problem though is we'd still need a way
for userspace to tell the kernel that all critical fs are mounted
as only userspace can know this for sure.

When is that done? How would the kernel know?

We do have PROBE_PREFER_ASYNCHRONOUS, but that does not provide any
guarantees over ready filesystems.

> At no point does it make sense to say "I have now mounted all the
> important filesystems". Maybe the firmware is extracted later by user
> space downloading it from the internet, and the module will then work
> only after that point"./
> 
> This whole "I have mounted important filesystems" is just pure and
> utter garbage. Stop pushing this shit.

Happy to do so, we do however need to document what we do expect users and
developers to do about this race for both drivers and built-in, keeping
in mind this is also for any users of kernel_read_file_from_path() as well.

  Luis

Powered by blists - more mailing lists