linux-kernel - Re: [PATCH v2 1/2] firmware: add nowarn variant of 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]

Date:   Mon, 19 Nov 2018 12:07:07 -0800
From:   Luis Chamberlain <mcgrof@...nel.org>
To:     Lucas Stach <l.stach@...gutronix.de>,
        Sebastian Reichel <sebastian.reichel@...labora.co.uk>
Cc:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Kees Cook <keescook@...omium.org>, mcgrof@...nel.org,
        Julia Lawall <julia.lawall@...6.fr>,
        linux-kernel@...r.kernel.org, dmaengine@...r.kernel.org,
        Vinod Koul <vkoul@...nel.org>, kernel@...gutronix.de,
        patchwork-lst@...gutronix.de
Subject: Re: [PATCH v2 1/2] firmware: add nowarn variant of
 request_firmware_nowait()

On Mon, Nov 12, 2018 at 05:01:42PM +0100, Lucas Stach wrote:
> Device drivers with optional firmware may still want to use the
> asynchronous firmware loading interface. To avoid printing a
> warining into the kernel log when the optional firmware is
> absent, add a nowarn variant of this interface.
> 
> Signed-off-by: Lucas Stach <l.stach@...gutronix.de>

Thanks for the patch Lucas!

> +EXPORT_SYMBOL(request_firmware_nowait_nowarn);

New symbols should use firmware_* prefix, so use:

  * firmware_request_nowait_nowarn()

Also, please make new functionality EXPORT_SYMBOL_GPL(), the old functioanlity
must be kept as-is, so in this caseEXPORT_SYMBOL().

Other than this, you should be aware that there has been significant
discussion over how to properly evolve the API of the firmware API since
last year, you may want to read those threads. The short and skinny of
it though is that the firmware API has two main diverging modes of
operation:

  o async
  o sync

The async functionality diverges from the synchronous functionality in
that it is data driven. The synchronous functionality is functional, and
experience shows that while data driven can avoid collateral evolutions
we *don't prefer it in the kernel*. So we should break down the async
API to match the sync functional design.

Internally we can use flags for small modifications, as we use them now,
but since we don't expose flags for the sync case lets try to keep
parity for this API then.

A good example of what we need to do. The uevent flag is only set to
false by only two drivers:

  o CONFIG_LEDS_LP55XX_COMMON
  o CONFIG_DELL_RBU

As such, this functionality should just be wrapped into its own single
functional call eventually.

The conversion of the async API to functional does not need to happen
for your changes, but new async API should follow the functional driven
approach. So please make your call work as functional.

Please let me know if this makes sense or if you have any quetsions!

[0] https://lore.kernel.org/lkml/20180628031332.GE21242@wotan.suse.de/T/#u
[1] https://lkml.kernel.org/r/20180421173650.GW14440@wotan.suse.de              
[2] https://lkml.kernel.org/r/20180422202609.GX14440@wotan.suse.de 

  Luis