linux-kernel - [PATCH 1/3] firmware: Avoid superfluous usermodehelper lock

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,  8 May 2013 08:56:35 +0200
From:	Takashi Iwai <tiwai@...e.de>
To:	linux-kernel@...r.kernel.org
Cc:	Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
	Ming Lei <ming.lei@...onical.com>
Subject: [PATCH 1/3] firmware: Avoid superfluous usermodehelper lock

When a firmware file can be loaded directly, there is no good reason
to lock usermodehelper.  It's needed only when the direct fw load
fails and falls back to the user-mode helper.

Signed-off-by: Takashi Iwai <tiwai@...e.de>
---
 drivers/base/firmware_class.c | 42 +++++++++++++++++++++++-------------------
 1 file changed, 23 insertions(+), 19 deletions(-)

diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c
index 4b1f926..9e1d39e 100644
--- a/drivers/base/firmware_class.c
+++ b/drivers/base/firmware_class.c
@@ -1006,7 +1006,7 @@ _request_firmware(const struct firmware **firmware_p, const char *name,
 		  struct device *device, bool uevent, bool nowait)
 {
 	struct firmware *fw;
-	long timeout;
+	bool usermode_locked = false;
 	int ret;
 
 	if (!firmware_p)
@@ -1017,31 +1017,35 @@ _request_firmware(const struct firmware **firmware_p, const char *name,
 		goto out;
 
 	ret = 0;
-	timeout = firmware_loading_timeout();
-	if (nowait) {
-		timeout = usermodehelper_read_lock_wait(timeout);
-		if (!timeout) {
-			dev_dbg(device, "firmware: %s loading timed out\n",
-				name);
-			ret = -EBUSY;
-			goto out;
-		}
-	} else {
-		ret = usermodehelper_read_trylock();
-		if (WARN_ON(ret)) {
-			dev_err(device, "firmware: %s will not be loaded\n",
-				name);
-			goto out;
+	if (!fw_get_filesystem_firmware(device, fw->priv)) {
+		long timeout = firmware_loading_timeout();
+		if (nowait) {
+			timeout = usermodehelper_read_lock_wait(timeout);
+			if (!timeout) {
+				dev_dbg(device, "firmware: %s loading timed out\n",
+					name);
+				ret = -EBUSY;
+				goto out;
+			}
+		} else {
+			ret = usermodehelper_read_trylock();
+			if (WARN_ON(ret)) {
+				dev_err(device, "firmware: %s will not be loaded\n",
+					name);
+				goto out;
+			}
 		}
-	}
 
-	if (!fw_get_filesystem_firmware(device, fw->priv))
+		usermode_locked = true;
 		ret = fw_load_from_user_helper(fw, name, device,
 					       uevent, nowait, timeout);
+	}
+
 	if (!ret)
 		ret = assign_firmware_buf(fw, device);
 
-	usermodehelper_read_unlock();
+	if (usermode_locked)
+		usermodehelper_read_unlock();
 
  out:
 	if (ret < 0) {
-- 
1.8.2.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/