From 06a45a93e7d34aab34522cc49479e1caecf3298c Mon Sep 17 00:00:00 2001 From: "Luis R. Rodriguez" Date: Tue, 2 May 2017 01:31:07 -0700 Subject: [PATCH] firmware: move umh try locks into the umh code This moves the usermode helper locks into only code paths that use the usermode helper API from the kernel. The usermode helper locks were originally added to prevent stalling suspend, later the firmware cache was added to help with this, and further later direct filesystem lookup was added by Linus to completely bypass udev due to the amount of issues the umh approach had. The usermode helper locks were kept even when the direct filesystem lookup mechanism is used though. A lot has changed since the original usermode helper locks were added but the recent commit which added the code for firmware_enabled() are intended to address any possible races cured only as collateral by using the locks as though side consequence of code evolution and this not being addressed any time sooner. With the firmware_enabled() code in place we are a bit more sure to move the usermode helper locks to UMH only code. There is a bit of history here so let's recap a bit of it to ensure nothing is lost and things are clear. The direct filesystem approach to loading firmware is rather new, it was added via commit abb139e75c2cdb ("firmware: teach the kernel to load firmware files directly from the filesystem") by Linus merged on the v3.7 release, to enable to bypass udev. usermodehelper_read_lock_wait() was added earlier via commit 9b78c1da60b3c ("firmware_class: Do not warn that system is not ready from async loads") merged on v3.4, after Rafael noted that the async firmware API call request_firmware_nowait() should not be penalized to fail if userspace is not available yet or frozen, it'd allow for a timeout grace period before giving up. The WARN_ON() was kept for the sync firmware API call though on request_firmware(). At this time there was no direct filesystem lookup for firmware though. The original usermode helper lock came from commit a144c6a6c924a ("PM: Print a warning if firmware is requested when tasks are frozen") merged on the v3.0 kernel by Rafael to print a warning back when firmware requests were used on resume(), thaw() or restore() callbacks and there was no direct fs lookups or the firmware cache. Signed-off-by: Luis R. Rodriguez Signed-off-by: Greg Kroah-Hartman --- drivers/base/firmware_class.c | 68 ++++++++++++++++++----------------- 1 file changed, 36 insertions(+), 32 deletions(-) diff --git a/drivers/base/firmware_class.c b/drivers/base/firmware_class.c index 7722943025f3..b9f907eedbf7 100644 --- a/drivers/base/firmware_class.c +++ b/drivers/base/firmware_class.c @@ -1102,23 +1102,51 @@ err_put_dev: static int fw_load_from_user_helper(struct firmware *firmware, const char *name, struct device *device, - unsigned int opt_flags, long timeout) + unsigned int opt_flags) { struct firmware_priv *fw_priv; + long timeout; + int ret; + + timeout = firmware_loading_timeout(); + if (opt_flags & FW_OPT_NOWAIT) { + timeout = usermodehelper_read_lock_wait(timeout); + if (!timeout) { + dev_dbg(device, "firmware: %s loading timed out\n", + name); + return -EBUSY; + } + } else { + ret = usermodehelper_read_trylock(); + if (WARN_ON(ret)) { + dev_err(device, "firmware: %s will not be loaded\n", + name); + return ret; + } + } fw_priv = fw_create_instance(firmware, name, device, opt_flags); - if (IS_ERR(fw_priv)) - return PTR_ERR(fw_priv); + if (IS_ERR(fw_priv)) { + ret = PTR_ERR(fw_priv); + goto out_unlock; + } fw_priv->buf = firmware->priv; - return _request_firmware_load(fw_priv, opt_flags, timeout); + ret = _request_firmware_load(fw_priv, opt_flags, timeout); + + if (!ret) + ret = assign_firmware_buf(firmware, device, opt_flags); + +out_unlock: + usermodehelper_read_unlock(); + + return ret; } #else /* CONFIG_FW_LOADER_USER_HELPER */ static inline int fw_load_from_user_helper(struct firmware *firmware, const char *name, - struct device *device, unsigned int opt_flags, - long timeout) + struct device *device, unsigned int opt_flags) { return -ENOENT; } @@ -1179,7 +1207,6 @@ _request_firmware(const struct firmware **firmware_p, const char *name, unsigned int opt_flags) { struct firmware *fw = NULL; - long timeout; int ret; if (!firmware_p) @@ -1200,25 +1227,6 @@ _request_firmware(const struct firmware **firmware_p, const char *name, goto out; } - ret = 0; - timeout = firmware_loading_timeout(); - if (opt_flags & FW_OPT_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; - } - } - ret = fw_get_filesystem_firmware(device, fw->priv); if (ret) { if (!(opt_flags & FW_OPT_NO_WARN)) @@ -1228,15 +1236,11 @@ _request_firmware(const struct firmware **firmware_p, const char *name, 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); + opt_flags); } - } - - if (!ret) + } else ret = assign_firmware_buf(fw, device, opt_flags); - usermodehelper_read_unlock(); - out: if (ret < 0) { release_firmware(fw); -- 2.20.1