diff options
Diffstat (limited to 'queue-2.6.32/pm-sleep-fix-freezer-failures-due-to-racy-usermodehelper_is_disabled.patch')
| -rw-r--r-- | queue-2.6.32/pm-sleep-fix-freezer-failures-due-to-racy-usermodehelper_is_disabled.patch | 178 |
1 files changed, 178 insertions, 0 deletions
diff --git a/queue-2.6.32/pm-sleep-fix-freezer-failures-due-to-racy-usermodehelper_is_disabled.patch b/queue-2.6.32/pm-sleep-fix-freezer-failures-due-to-racy-usermodehelper_is_disabled.patch new file mode 100644 index 0000000..06d5d16 --- /dev/null +++ b/queue-2.6.32/pm-sleep-fix-freezer-failures-due-to-racy-usermodehelper_is_disabled.patch @@ -0,0 +1,178 @@ +From srivatsa.bhat@linux.vnet.ibm.com Thu Mar 1 13:32:07 2012 +From: "Srivatsa S. Bhat" <srivatsa.bhat@linux.vnet.ibm.com> +Date: Wed, 29 Feb 2012 12:24:01 +0530 +Subject: PM / Sleep: Fix freezer failures due to racy usermodehelper_is_disabled() +To: gregkh@linuxfoundation.org +Cc: stable@vger.kernel.org, rjw@sisk.pl, valdis.kletnieks@vt.edu, cloos@hjcloos.com, riesebie@lxtec.de, torvalds@linux-foundation.org, penguin-kernel@i-love.sakura.ne.jp, srivatsa.bhat@linux.vnet.ibm.com +Message-ID: <20120229065327.4761.48161.stgit@srivatsabhat.in.ibm.com> + + +From: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com> + +[ Upstream commit b298d289c79211508f11cb50749b0d1d54eb244a ] + +Commit a144c6a (PM: Print a warning if firmware is requested when tasks +are frozen) introduced usermodehelper_is_disabled() to warn and exit +immediately if firmware is requested when usermodehelpers are disabled. + +However, it is racy. Consider the following scenario, currently used in +drivers/base/firmware_class.c: + +... +if (usermodehelper_is_disabled()) + goto out; + +/* Do actual work */ +... + +out: + return err; + +Nothing prevents someone from disabling usermodehelpers just after the check +in the 'if' condition, which means that it is quite possible to try doing the +"actual work" with usermodehelpers disabled, leading to undesirable +consequences. + +In particular, this race condition in _request_firmware() causes task freezing +failures whenever suspend/hibernation is in progress because, it wrongly waits +to get the firmware/microcode image from userspace when actually the +usermodehelpers are disabled or userspace has been frozen. +Some of the example scenarios that cause freezing failures due to this race +are those that depend on userspace via request_firmware(), such as x86 +microcode module initialization and microcode image reload. + +Previous discussions about this issue can be found at: +http://thread.gmane.org/gmane.linux.kernel/1198291/focus=1200591 + +This patch adds proper synchronization to fix this issue. + +It is to be noted that this patchset fixes the freezing failures but doesn't +remove the warnings. IOW, it does not attempt to add explicit synchronization +to x86 microcode driver to avoid requesting microcode image at inopportune +moments. Because, the warnings were introduced to highlight such cases, in the +first place. And we need not silence the warnings, since we take care of the +*real* problem (freezing failure) and hence, after that, the warnings are +pretty harmless anyway. + +Signed-off-by: Srivatsa S. Bhat <srivatsa.bhat@linux.vnet.ibm.com> +Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl> +Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> +--- + + drivers/base/firmware_class.c | 3 +++ + include/linux/kmod.h | 4 ++++ + kernel/kmod.c | 24 +++++++++++++++++++++++- + 3 files changed, 30 insertions(+), 1 deletion(-) + +--- a/drivers/base/firmware_class.c ++++ b/drivers/base/firmware_class.c +@@ -508,6 +508,8 @@ _request_firmware(const struct firmware + return 0; + } + ++ read_lock_usermodehelper(); ++ + if (WARN_ON(usermodehelper_is_disabled())) { + dev_err(device, "firmware: %s will not be loaded\n", name); + retval = -EBUSY; +@@ -551,6 +553,7 @@ error_kfree_fw: + kfree(firmware); + *firmware_p = NULL; + out: ++ read_unlock_usermodehelper(); + return retval; + } + +--- a/include/linux/kmod.h ++++ b/include/linux/kmod.h +@@ -108,8 +108,12 @@ extern int call_usermodehelper_pipe(char + extern int usermodehelper_disable(void); + extern void usermodehelper_enable(void); + extern bool usermodehelper_is_disabled(void); ++extern void read_lock_usermodehelper(void); ++extern void read_unlock_usermodehelper(void); + #else + static inline bool usermodehelper_is_disabled(void) { return false; } ++static inline void read_lock_usermodehelper(void) {} ++static inline void read_unlock_usermodehelper(void) {} + #endif + + #endif /* __LINUX_KMOD_H__ */ +--- a/kernel/kmod.c ++++ b/kernel/kmod.c +@@ -35,6 +35,7 @@ + #include <linux/resource.h> + #include <linux/notifier.h> + #include <linux/suspend.h> ++#include <linux/rwsem.h> + #include <asm/uaccess.h> + + #include <trace/events/module.h> +@@ -43,6 +44,8 @@ extern int max_threads; + + static struct workqueue_struct *khelper_wq; + ++static DECLARE_RWSEM(umhelper_sem); ++ + #ifdef CONFIG_MODULES + + /* +@@ -286,6 +289,7 @@ static void __call_usermodehelper(struct + * If set, call_usermodehelper_exec() will exit immediately returning -EBUSY + * (used for preventing user land processes from being created after the user + * land has been frozen during a system-wide hibernation or suspend operation). ++ * Should always be manipulated under umhelper_sem acquired for write. + */ + static int usermodehelper_disabled; + +@@ -304,6 +308,18 @@ static DECLARE_WAIT_QUEUE_HEAD(running_h + */ + #define RUNNING_HELPERS_TIMEOUT (5 * HZ) + ++void read_lock_usermodehelper(void) ++{ ++ down_read(&umhelper_sem); ++} ++EXPORT_SYMBOL_GPL(read_lock_usermodehelper); ++ ++void read_unlock_usermodehelper(void) ++{ ++ up_read(&umhelper_sem); ++} ++EXPORT_SYMBOL_GPL(read_unlock_usermodehelper); ++ + /** + * usermodehelper_disable - prevent new helpers from being started + */ +@@ -311,8 +327,10 @@ int usermodehelper_disable(void) + { + long retval; + ++ down_write(&umhelper_sem); + usermodehelper_disabled = 1; +- smp_mb(); ++ up_write(&umhelper_sem); ++ + /* + * From now on call_usermodehelper_exec() won't start any new + * helpers, so it is sufficient if running_helpers turns out to +@@ -325,7 +343,9 @@ int usermodehelper_disable(void) + if (retval) + return 0; + ++ down_write(&umhelper_sem); + usermodehelper_disabled = 0; ++ up_write(&umhelper_sem); + return -EAGAIN; + } + +@@ -334,7 +354,9 @@ int usermodehelper_disable(void) + */ + void usermodehelper_enable(void) + { ++ down_write(&umhelper_sem); + usermodehelper_disabled = 0; ++ up_write(&umhelper_sem); + } + + /** |