linux-kernel - Re: [PATCH v3 05/20] firmware: simplify CONFIG_FW_LOADER_USER_HELPER

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, 14 Mar 2018 15:36:32 -0700
From:   "Luis R. Rodriguez" <mcgrof@...nel.org>
To:     Greg KH <gregkh@...uxfoundation.org>
Cc:     Andrew Morton <akpm@...ux-foundation.org>,
        cantabile.desu@...il.com, kubakici@...pl,
        linux-wireless <linux-wireless@...r.kernel.org>,
        Kees Cook <keescook@...omium.org>, shuah@...nel.org,
        Martin Fuzzey <mfuzzey@...keon.com>,
        Mimi Zohar <zohar@...ux.vnet.ibm.com>,
        David Howells <dhowells@...hat.com>,
        Pali Rohár <pali.rohar@...il.com>,
        Takashi Iwai <tiwai@...e.de>,
        Arend Van Spriel <arend.vanspriel@...adcom.com>,
        Rafał Miłecki <zajec5@...il.com>,
        Nicolas Broeking <nbroeking@...com>,
        Vikram Mulukutla <markivx@...eaurora.org>,
        Mark Brown <broonie@...nel.org>,
        Dmitry Torokhov <dmitry.torokhov@...il.com>,
        David Woodhouse <dwmw2@...radead.org>,
        Linus Torvalds <torvalds@...ux-foundation.org>,
        Abhay Salunke <Abhay_Salunke@...l.com>,
        Bjorn Andersson <bjorn.andersson@...aro.org>,
        jewalt@...innovations.com, oneukum@...e.com,
        Alexei Starovoitov <ast@...com>,
        Andres Rodriguez <andresx7@...il.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Linux FS Devel <linux-fsdevel@...r.kernel.org>
Subject: Re: [PATCH v3 05/20] firmware: simplify CONFIG_FW_LOADER_USER_HELPER_FALLBACK
 further

On Wed, Mar 14, 2018 at 11:53 AM, Greg KH <gregkh@...uxfoundation.org> wrote:
> On Sat, Mar 10, 2018 at 06:14:46AM -0800, Luis R. Rodriguez wrote:
>> All CONFIG_FW_LOADER_USER_HELPER_FALLBACK really is, is just a bool,
>> initailized at build time. Define it as such. This simplifies the
>> logic even further, removing now all explicit #ifdefs around the code.
>>
>> Acked-by: Kees Cook <keescook@...omium.org>
>> Signed-off-by: Luis R. Rodriguez <mcgrof@...nel.org>
>> ---
>>  drivers/base/firmware_loader.c | 25 ++++++++++++++++++-------
>>  1 file changed, 18 insertions(+), 7 deletions(-)
>>
>> diff --git a/drivers/base/firmware_loader.c b/drivers/base/firmware_loader.c
>> index 7dd36ace6152..59dba794ce1a 100644
>> --- a/drivers/base/firmware_loader.c
>> +++ b/drivers/base/firmware_loader.c
>> @@ -266,6 +266,22 @@ static inline bool fw_state_is_aborted(struct fw_priv *fw_priv)
>>
>>  #ifdef CONFIG_FW_LOADER_USER_HELPER
>>
>> +/**
>> + * struct firmware_fallback_config - firmware fallback configuratioon settings
>> + *
>> + * Helps describe and fine tune the fallback mechanism.
>> + *
>> + * @force_sysfs_fallback: force the sysfs fallback mechanism to be used
>> + *   as if one had enabled CONFIG_FW_LOADER_USER_HELPER_FALLBACK=y.
>> + */
>> +struct firmware_fallback_config {
>> +     bool force_sysfs_fallback;
>> +};
>
> This is C, why are you messing around with a structure and "getters and
> setters" for a set of simple values?
>
>> +static const struct firmware_fallback_config fw_fallback_config = {
>> +     .force_sysfs_fallback = IS_ENABLED(CONFIG_FW_LOADER_USER_HELPER_FALLBACK),
>> +};
>
> static bool force_sysfs_fallback = IS_ENABLED(CONFIG_FW_LOADER_USER_HELPER_FALLBACK);

For this case yes, but I later expand on this structure for all
fallback configuration items. So we have to start it somewhere.

> Then just read/write the foolish thing, don't make this more complex
> than is needed please.

Please read the final results and let me know what you think.

> There is a "getter and setters are evil" rant somewhere out there
> online, if you really need me to dig up the old tired arguments :)

I don't use them for sport, I use them given the fallback stuff *is*
not something the direct core firmware code should use openly, it will
really depend on if the fallback interface was enabled. As such we
provide wrapper for access into them.

  Luis