linux-kernel - [PATCH v2] firmware loader: inform direct failure when udev loader is disabled

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-next>] [day] [month] [year] [list]
Message-Id: <1404270473-4285-1-git-send-email-mcgrof@do-not-panic.com>
Date:	Tue,  1 Jul 2014 20:07:53 -0700
From:	"Luis R. Rodriguez" <mcgrof@...not-panic.com>
To:	ming.lei@...onical.com, gregkh@...uxfoundation.org
Cc:	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>, Takashi Iwai <tiwai@...e.de>
Subject: [PATCH v2] firmware loader: inform direct failure when udev loader is disabled

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>
---

This v2 modifies the error to always be called and only in the
request_firmware_direct case do we not send smoke signals.

 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..60d0e53 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_DIRECT_ONLY (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_DIRECT_ONLY))
 			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_DIRECT_ONLY);
 	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/