linux-kernel - [RFC] firmware loader: retry _nowait requests when userhelper is not yet available

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]

Date:	Sat, 3 Mar 2012 21:22:36 +0100
From:	Christian Lamparter <chunkeey@...glemail.com>
To:	linux-kernel@...r.kernel.org
Cc:	gregkh@...uxfoundation.org, chunkeey@...glemail.com
Subject: [RFC] firmware loader: retry _nowait requests when userhelper is not yet available

During resume, the userhelper might not be available. However for
drivers which use the request_firmware_nowait interface, this will
only lead to a pointless WARNING and a device which no longer works
after the resume [since it couldn't get the firmware, because the
userhelper was not available to take the request].

In order to solve this "chicken or egg" dilemma, the code now
retries _nowait requests at one second intervals until the
"loading_timeout" time is up.

---
I'm aware about the previous "request_firmware* in probe" discussions.
Unfortunately, the hardware needs firmware so there is no other way
around it. So please, I just wanted to know what the general opinion
about the idea behind this patch is.

Regards,
	Christian

BTW: I'm not on the list, so please keep the 'CC'.
---
 drivers/base/firmware_class.c |   24 +++++++++++++++++++++++-
 1 files changed, 23 insertions(+), 1 deletions(-)

diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c
index 6c9387d..9f70096 100644
--- a/drivers/base/firmware_class.c
+++ b/drivers/base/firmware_class.c
@@ -19,6 +19,7 @@
 #include <linux/kthread.h>
 #include <linux/highmem.h>
 #include <linux/firmware.h>
+#include <linux/delay.h>
 #include <linux/slab.h>
 
 #define to_dev(obj) container_of(obj, struct device, kobj)
@@ -535,6 +536,11 @@ static int _request_firmware(const struct firmware **firmware_p,
 
 	read_lock_usermodehelper();
 
+	if (nowait && usermodehelper_is_disabled()) {
+		retval = -EBUSY;
+		goto out;
+	}
+
 	if (WARN_ON(usermodehelper_is_disabled())) {
 		dev_err(device, "firmware: %s will not be loaded\n", name);
 		retval = -EBUSY;
@@ -633,7 +639,7 @@ static int request_firmware_work_func(void *arg)
 {
 	struct firmware_work *fw_work = arg;
 	const struct firmware *fw;
-	int ret;
+	int ret, timeout = loading_timeout;
 
 	if (!arg) {
 		WARN_ON(1);
@@ -642,6 +648,22 @@ static int request_firmware_work_func(void *arg)
 
 	ret = _request_firmware(&fw, fw_work->name, fw_work->device,
 				fw_work->uevent, true);
+
+	while (ret == -EBUSY) {
+		/*
+		 * Try to retrieve the firmware within the loading timeout.
+		 * To stick with the loading timeout convention from above:
+		 *	loading_timeout = 0 means 'try forever' as well.
+		 */
+
+		msleep(1000);
+		ret = _request_firmware(&fw, fw_work->name, fw_work->device,
+					fw_work->uevent, true);
+
+		if (timeout != 0 && timeout-- == 1)
+			break;
+	};
+
 	fw_work->cont(fw, fw_work->context);
 
 	module_put(fw_work->module);
-- 
1.7.9.1

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