linux-kernel - Re: [PATCH V5 1/2] firmware: add more flexible request_firmware

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: <20170810170738.GD27873@wotan.suse.de>
Date:   Thu, 10 Aug 2017 19:07:38 +0200
From:   "Luis R. Rodriguez" <mcgrof@...nel.org>
To:     "Coelho, Luciano" <luciano.coelho@...el.com>
Cc:     "kvalo@...eaurora.org" <kvalo@...eaurora.org>,
        "mcgrof@...nel.org" <mcgrof@...nel.org>,
        "pieter-paul.giesberts@...adcom.com" 
        <pieter-paul.giesberts@...adcom.com>,
        "bjorn.andersson@...aro.org" <bjorn.andersson@...aro.org>,
        "arend.vanspriel@...adcom.com" <arend.vanspriel@...adcom.com>,
        "hante.meuleman@...adcom.com" <hante.meuleman@...adcom.com>,
        "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
        "keescook@...omium.org" <keescook@...omium.org>,
        "linux-wireless@...r.kernel.org" <linux-wireless@...r.kernel.org>,
        "alan@...ux.intel.com" <alan@...ux.intel.com>,
        "moritz.fischer@...us.com" <moritz.fischer@...us.com>,
        "pjones@...hat.com" <pjones@...hat.com>,
        "wagi@...om.org" <wagi@...om.org>,
        "pmladek@...e.com" <pmladek@...e.com>,
        "atull@...nel.org" <atull@...nel.org>,
        "yi1.li@...ux.intel.com" <yi1.li@...ux.intel.com>,
        "wright.feng@...ress.com" <wright.feng@...ress.com>,
        "torvalds@...ux-foundation.org" <torvalds@...ux-foundation.org>,
        "netdev@...r.kernel.org" <netdev@...r.kernel.org>,
        "luto@...nel.org" <luto@...nel.org>,
        "dwmw2@...radead.org" <dwmw2@...radead.org>,
        "takahiro.akashi@...aro.org" <takahiro.akashi@...aro.org>,
        "rjw@...ysocki.net" <rjw@...ysocki.net>,
        "hdegoede@...hat.com" <hdegoede@...hat.com>,
        "rafal@...ecki.pl" <rafal@...ecki.pl>,
        "Berg, Johannes" <johannes.berg@...el.com>,
        "zajec5@...il.com" <zajec5@...il.com>,
        "tytso@....edu" <tytso@....edu>,
        "dhowells@...hat.com" <dhowells@...hat.com>,
        "Grumbach, Emmanuel" <emmanuel.grumbach@...el.com>,
        "chi-hsien.lin@...ress.com" <chi-hsien.lin@...ress.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "franky.lin@...adcom.com" <franky.lin@...adcom.com>
Subject: Re: [PATCH V5 1/2] firmware: add more flexible
 request_firmware_async function

On Thu, Aug 03, 2017 at 05:55:18AM +0000, Coelho, Luciano wrote:
> On Thu, 2017-08-03 at 08:23 +0300, Kalle Valo wrote:
> > "Luis R. Rodriguez" <mcgrof@...nel.org> writes:
> > 
> > > > +int request_firmware_nowait(struct module *module, bool uevent,
> > > > +			    const char *name, struct device *device, gfp_t gfp,
> > > > +			    void *context,
> > > > +			    void (*cont)(const struct firmware *fw, void *context))
> > > > +{
> > > > +	unsigned int opt_flags = FW_OPT_FALLBACK |
> > > > +		(uevent ? FW_OPT_UEVENT : FW_OPT_USERHELPER);
> > > > +
> > > > +	return __request_firmware_nowait(module, opt_flags, name, device, gfp,
> > > > +					 context, cont);
> > > > +}
> > > >  EXPORT_SYMBOL(request_firmware_nowait);
> > > >  
> > > > +int __request_firmware_async(struct module *module, const char *name,
> > > > +			     struct firmware_opts *fw_opts, struct device *dev,
> > > > +			     void *context,
> > > > +			     void (*cont)(const struct firmware *fw, void *context))
> > > > +{
> > > > +	unsigned int opt_flags = FW_OPT_UEVENT;
> > > 
> > > This exposes a long issue. Think -- why do we want this enabled by default? Its
> > > actually because even though the fallback stuff is optional and can be, the uevent
> > > internal flag *also* provides caching support as a side consequence only. We
> > > don't want to add a new API without first cleaning up that mess.
> > > 
> > > This is a slipery slope and best to clean that up before adding any new API.
> > > 
> > > That and also Greg recently stated he would like to see at least 3 users of
> > > a feature before adding it. Although I think that's pretty arbitrary, and
> > > considering that request_firmware_into_buf() only has *one* user -- its what
> > > he wishes.
> > 
> > ath10k at least needs a way to silence the warning for missing firmware
> > and I think iwlwifi also.
> 
> Yes, iwlwifi needs to silence the warning.  It the feature (only one,
> really) that I've been waiting for.

Luca,

can you confirm? The API revision thing is one thing but as I noted to
Kalle that can be done using a different API scheme as I had proposed on
the driver data API.

Other than that are there specific firmware requests which are async which
are not API revision style (min...max) for which an async request is optional?

 Luis