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
| ||
|
Date: Fri, 04 Jul 2014 07:52:07 +0200 From: Takashi Iwai <tiwai@...e.de> To: "Luis R. Rodriguez" <mcgrof@...not-panic.com> Cc: ming.lei@...onical.com, gregkh@...uxfoundation.org, linux-kernel@...r.kernel.org, "Luis R. Rodriguez" <mcgrof@...e.com>, Tom Gundersen <teg@...m.no>, Abhay Salunke <Abhay_Salunke@...l.com>, Stefan Roese <sr@...x.de>, Arnd Bergmann <arnd@...db.de>, Kay Sievers <kay@...y.org> Subject: Re: [PATCH v3] firmware loader: inform direct failure when udev loader is disabled At Wed, 2 Jul 2014 09:55:05 -0700, Luis R. Rodriguez wrote: > > From: "Luis R. Rodriguez" <mcgrof@...e.com> > > Now that the udev firmware loader is optional request_firmware() > will not provide any information on the kernel ring buffer if > direct firmware loading failed and udev firmware loading is disabled. > If no information is needed request_firmware_direct() should be used > for optional firmware, at which point drivers can take on the onus > over informing of any failures, if udev firmware loading is disabled > though we should at the very least provide some sort of information > as when the udev loader was enabled by default back in the days. > > With this change with a simple firmware load test module [0]: > > Example output without FW_LOADER_USER_HELPER_FALLBACK > > platform fake-dev.0: Direct firmware load for fake.bin failed > with error -2 > > Example with FW_LOADER_USER_HELPER_FALLBACK > > platform fake-dev.0: Direct firmware load for fake.bin failed with error -2 > platform fake-dev.0: Falling back to user helper > > Without this change without FW_LOADER_USER_HELPER_FALLBACK we > get no output logged upon failure. > > Cc: Tom Gundersen <teg@...m.no> > Cc: Ming Lei <ming.lei@...onical.com> > Cc: Greg Kroah-Hartman <gregkh@...uxfoundation.org> > Cc: Abhay Salunke <Abhay_Salunke@...l.com> > Cc: Stefan Roese <sr@...x.de> > Cc: Arnd Bergmann <arnd@...db.de> > Cc: Kay Sievers <kay@...y.org> > Cc: Takashi Iwai <tiwai@...e.de> > Signed-off-by: Luis R. Rodriguez <mcgrof@...e.com> > --- > > Use FW_OPT_NO_WARN instead. Looks good to me. Reviewed-by: Takashi Iwai <tiwai@...e.de> thanks, Takashi > > drivers/base/firmware_class.c | 13 +++++++------ > include/linux/firmware.h | 15 ++++++++------- > 2 files changed, 15 insertions(+), 13 deletions(-) > > diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c > index 46ea5f4..08e67cc 100644 > --- a/drivers/base/firmware_class.c > +++ b/drivers/base/firmware_class.c > @@ -109,6 +109,7 @@ static inline long firmware_loading_timeout(void) > #else > #define FW_OPT_FALLBACK 0 > #endif > +#define FW_OPT_NO_WARN (1U << 3) > > struct firmware_cache { > /* firmware_buf instance will be added into the below list */ > @@ -1116,10 +1117,11 @@ _request_firmware(const struct firmware **firmware_p, const char *name, > > ret = fw_get_filesystem_firmware(device, fw->priv); > if (ret) { > - if (opt_flags & FW_OPT_USERHELPER) { > + if (!(opt_flags & FW_OPT_NO_WARN)) > dev_warn(device, > - "Direct firmware load failed with error %d\n", > - ret); > + "Direct firmware load for %s failed with error %d\n", > + name, ret); > + if (opt_flags & FW_OPT_USERHELPER) { > dev_warn(device, "Falling back to user helper\n"); > ret = fw_load_from_user_helper(fw, name, device, > opt_flags, timeout); > @@ -1176,7 +1178,6 @@ request_firmware(const struct firmware **firmware_p, const char *name, > } > EXPORT_SYMBOL(request_firmware); > > -#ifdef CONFIG_FW_LOADER_USER_HELPER_FALLBACK > /** > * request_firmware: - load firmware directly without usermode helper > * @firmware_p: pointer to firmware image > @@ -1193,12 +1194,12 @@ int request_firmware_direct(const struct firmware **firmware_p, > { > int ret; > __module_get(THIS_MODULE); > - ret = _request_firmware(firmware_p, name, device, FW_OPT_UEVENT); > + ret = _request_firmware(firmware_p, name, device, > + FW_OPT_UEVENT | FW_OPT_NO_WARN); > module_put(THIS_MODULE); > return ret; > } > EXPORT_SYMBOL_GPL(request_firmware_direct); > -#endif > > /** > * release_firmware: - release the resource associated with a firmware image > diff --git a/include/linux/firmware.h b/include/linux/firmware.h > index 67e5b80..5c41c5e 100644 > --- a/include/linux/firmware.h > +++ b/include/linux/firmware.h > @@ -45,6 +45,8 @@ 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)); > +int request_firmware_direct(const struct firmware **fw, const char *name, > + struct device *device); > > void release_firmware(const struct firmware *fw); > #else > @@ -66,13 +68,12 @@ static inline void release_firmware(const struct firmware *fw) > { > } > > -#endif > +static inline int request_firmware_direct(const struct firmware **fw, > + const char *name, > + struct device *device) > +{ > + return -EINVAL; > +} > > -#ifdef CONFIG_FW_LOADER_USER_HELPER_FALLBACK > -int request_firmware_direct(const struct firmware **fw, const char *name, > - struct device *device); > -#else > -#define request_firmware_direct request_firmware > #endif > - > #endif > -- > 2.0.0 > -- To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@...r.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists