linux-kernel - Re: [PATCH 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] [thread-next>] [day] [month] [year] [list]

Message-ID: <20170221175948.GK31264@wotan.suse.de>
Date:   Tue, 21 Feb 2017 18:59:48 +0100
From:   "Luis R. Rodriguez" <mcgrof@...nel.org>
To:     Rafał Miłecki <zajec5@...il.com>
Cc:     Ming Lei <ming.lei@...onical.com>,
        "Luis R . Rodriguez" <mcgrof@...nel.org>,
        Greg KH <gregkh@...uxfoundation.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Kalle Valo <kvalo@...eaurora.org>,
        Arend van Spriel <arend.vanspriel@...adcom.com>,
        linux-wireless@...r.kernel.org,
        brcm80211-dev-list.pdl@...adcom.com,
        Rafał Miłecki <rafal@...ecki.pl>
Subject: Re: [PATCH 1/2] firmware: add more flexible request_firmware_async
 function

On Wed, Feb 15, 2017 at 11:29:47PM +0100, Rafał Miłecki wrote:
> From: Rafał Miłecki <rafal@...ecki.pl>
> 
> So far we got only one function for loading firmware asynchronously:
> request_firmware_nowait. It didn't allow much customization of firmware
> loading process - there is only one bool uevent argument. Moreover this
> bool also controls user helper in an unclear way.
> 
> Resolve this problem by adding a one flexible function 

You've just taken under-the-hood flags and exposed them without considering and
enabling easy testing of any possible conflicts here.  Take for instance the
FW_OPT_NOCACHE feature -- not using caching functionality has implications for
driver developers -- they *must* resolve their own suspend/resume mishaps.
Another example, when we get firmware signing having just flags won't cut it
for optional parameters, we want a struct with the ability to specify custom
signing requirements. Exposing just flags will not cut it for us long term and
we'd have to then either add new export symbol or modify this one.

This is not as flexible nor as well documented as I want for future
functionality. Nor is there any series of battery of tests of all possible
options to ensure we do not regress. I've addressed this in the driver data
series.

> and making old
> request_firmware_nowait a simple inline using new solution. 

This I had not addressed in the driver data series but I will fold similar
strategy onto it.

> This
> implementation:
> 1) Modifies only single bits on existing code
> 2) Doesn't require adjusting / rewriting current drivers
> 3) Adds new function for drivers that need more control over loading a
>    firmware. Thanks to using flags more features can be added later.

As I noted in the driver data series -- we're passed using the firmware API for
non-firmware specific stuff, and as recent history shows regressions are are
easy, specially with the fallback mechanism. A new API which just gives us
2 calls: sync/async calls and shares a common set of optional parameters is
what we need, we need proper testing to ensure we also don't regress should
new features be introduced.

What I'll do is I'll integrate the feature you are asking for here and fold
this into the driver data series as what we need now is actual users of new
functionality, not just a test driver.

  Luis