[PATCH] Close race with preempt and modular pm_idle callbacks

The following patch from Shaohua Li fixes a race with preempt enabled when
a module containing a pm_idle callback is unloaded.  Cached values in local
variables need to be protected as RCU critical sections so that the
synchronize_kernel() call in the unload path waits for all processors.
There original bugzilla entry can be found at

Shaohua, i had to make a small change (variable declaration after code in
code block) so that it compiles with geriatric compilers such as the ones
Andrew is attached to ;)

http://bugzilla.kernel.org/show_bug.cgi?id=1716

Signed-off-by: Li Shaohua <shaohua.li@intel.com>
Signed-off-by: Zwane Mwaikambo <zwane@linuxpower.ca>
Signed-off-by: Andrew Morton <akpm@osdl.org>
Signed-off-by: Linus Torvalds <torvalds@osdl.org>

diff --git a/arch/i386/kernel/apm.c b/arch/i386/kernel/apm.c
index 4cafb2e99ef4286bed01fdbeffc4d41c41b5340e..d9a80ba232e05b2b4b56544658f4af8bd5923926 100644 (file)
--- a/arch/i386/kernel/apm.c
+++ b/arch/i386/kernel/apm.c
@@ -2362,8 +2362,15 @@ static void __exit apm_exit(void)
 {
        int     error;
 
-       if (set_pm_idle)
+       if (set_pm_idle) {
                pm_idle = original_pm_idle;
+               /*
+                * We are about to unload the current idle thread pm callback
+                * (pm_idle), Wait for all processors to update cached/local
+                * copies of pm_idle before proceeding.
+                */
+               synchronize_kernel();
+       }
        if (((apm_info.bios.flags & APM_BIOS_DISENGAGED) == 0)
            && (apm_info.connection_version > 0x0100)) {
                error = apm_engage_power_management(APM_DEVICE_ALL, 0);

diff --git a/arch/i386/kernel/process.c b/arch/i386/kernel/process.c
index 49a15cc249411265eea8303a479e0a853cdecbcd..0095fa1dda6f6452f7a51fa08333458f0f75a6d0 100644 (file)
--- a/arch/i386/kernel/process.c
+++ b/arch/i386/kernel/process.c
@@ -142,13 +142,21 @@ void cpu_idle (void)
        /* endless idle loop with no priority at all */
        while (1) {
                while (!need_resched()) {
-                       void (*idle)(void) = pm_idle;
+                       void (*idle)(void);
+                       /*
+                        * Mark this as an RCU critical section so that
+                        * synchronize_kernel() in the unload path waits
+                        * for our completion.
+                        */
+                       rcu_read_lock();
+                       idle = pm_idle;
 
                        if (!idle)
                                idle = default_idle;
 
                        irq_stat[smp_processor_id()].idle_timestamp = jiffies;
                        idle();
+                       rcu_read_unlock();
                }
                schedule();
        }

diff --git a/arch/ia64/kernel/process.c b/arch/ia64/kernel/process.c
index 842c95b8a751cd74f480ce24ccbad37a704329cb..70496304e9a27c6966f2f0afa1b91b9ed8df33e7 100644 (file)
--- a/arch/ia64/kernel/process.c
+++ b/arch/ia64/kernel/process.c
@@ -228,18 +228,26 @@ cpu_idle (void *unused)
 
        /* endless idle loop with no priority at all */
        while (1) {
-               void (*idle)(void) = pm_idle;
-               if (!idle)
-                       idle = default_idle;
-
 #ifdef CONFIG_SMP
                if (!need_resched())
                        min_xtp();
 #endif
                while (!need_resched()) {
+                       void (*idle)(void);
+
                        if (mark_idle)
                                (*mark_idle)(1);
+                       /*
+                        * Mark this as an RCU critical section so that
+                        * synchronize_kernel() in the unload path waits
+                        * for our completion.
+                        */
+                       rcu_read_lock();
+                       idle = pm_idle;
+                       if (!idle)
+                               idle = default_idle;
                        (*idle)();
+                       rcu_read_unlock();
                }
 
                if (mark_idle)

diff --git a/arch/x86_64/kernel/process.c b/arch/x86_64/kernel/process.c
index cd22f3c11a08585f9dbb8b838a8c3cd8fa9e31c0..6e835be5f26a6aba9da3ffc878e3419599927513 100644 (file)
--- a/arch/x86_64/kernel/process.c
+++ b/arch/x86_64/kernel/process.c
@@ -130,11 +130,20 @@ void cpu_idle (void)
 {
        /* endless idle loop with no priority at all */
        while (1) {
-               void (*idle)(void) = pm_idle;
-               if (!idle)
-                       idle = default_idle;
-               while (!need_resched())
+               while (!need_resched()) {
+                       void (*idle)(void);
+                       /*
+                        * Mark this as an RCU critical section so that
+                        * synchronize_kernel() in the unload path waits
+                        * for our completion.
+                        */
+                       rcu_read_lock();
+                       idle = pm_idle;
+                       if (!idle)
+                               idle = default_idle;
                        idle();
+                       rcu_read_unlock();
+               }
                schedule();
        }
 }

diff --git a/drivers/acpi/processor.c b/drivers/acpi/processor.c
index 1b830bb051051ef2a73f956b0879193707ee81cb..83fb743aa3e70996887edde7ca2917f315540e14 100644 (file)
--- a/drivers/acpi/processor.c
+++ b/drivers/acpi/processor.c
@@ -2419,6 +2419,11 @@ acpi_processor_remove (
        /* Unregister the idle handler when processor #0 is removed. */
        if (pr->id == 0) {
                pm_idle = pm_idle_save;
+               /*
+                * We are about to unload the current idle thread pm callback
+                * (pm_idle), Wait for all processors to update cached/local
+                * copies of pm_idle before proceeding.
+                */
                synchronize_kernel();
        }