freezer: close potential race between refrigerator and thaw_tasks

If the freezing of tasks fails and a task is preempted in refrigerator()
before calling frozen_process(), then thaw_tasks() may run before this task is
frozen.  In that case the task will freeze and no one will thaw it.

To fix this race we can call freezing(current) in refrigerator() along with
frozen_process(current) under the task_lock() which also should be taken in
the error path of try_to_freeze_tasks() as well as in thaw_process().
Moreover, if thaw_process() additionally clears TIF_FREEZE for tasks that are
not frozen, we can be sure that all tasks are thawed and there are no pending
"freeze" requests after thaw_tasks() has run.

Signed-off-by: Rafael J. Wysocki <rjw@sisk.pl>
Acked-by: Pavel Machek <pavel@ucw.cz>
Cc: Gautham R Shenoy <ego@in.ibm.com>
Cc: Oleg Nesterov <oleg@tv-sign.ru>
Cc: "Eric W. Biederman" <ebiederm@xmission.com>
Signed-off-by: Andrew Morton <akpm@linux-foundation.org>
Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

diff --git a/include/linux/freezer.h b/include/linux/freezer.h
index 5e75e26d4787248caf175063afad75010824672a..db5423eae24d8dd511bbd88714b5264b45b4ba19 100644 (file)
--- a/include/linux/freezer.h
+++ b/include/linux/freezer.h
@@ -37,14 +37,24 @@ static inline void do_not_freeze(struct task_struct *p)
 
 /*
  * Wake up a frozen process
+ *
+ * task_lock() is taken to prevent the race with refrigerator() which may
+ * occur if the freezing of tasks fails.  Namely, without the lock, if the
+ * freezing of tasks failed, thaw_tasks() might have run before a task in
+ * refrigerator() could call frozen_process(), in which case the task would be
+ * frozen and no one would thaw it.
  */
 static inline int thaw_process(struct task_struct *p)
 {
+       task_lock(p);
        if (frozen(p)) {
                p->flags &= ~PF_FROZEN;
+               task_unlock(p);
                wake_up_process(p);
                return 1;
        }
+       clear_tsk_thread_flag(p, TIF_FREEZE);
+       task_unlock(p);
        return 0;
 }
 

diff --git a/kernel/power/process.c b/kernel/power/process.c
index 088419387388ca181d6e855a8aa98be97e930025..02e490e311ebad49b60fcf7b1eeba0dcd1d68cc3 100644 (file)
--- a/kernel/power/process.c
+++ b/kernel/power/process.c
@@ -37,10 +37,18 @@ void refrigerator(void)
        /* Hmm, should we be allowed to suspend when there are realtime
           processes around? */
        long save;
+
+       task_lock(current);
+       if (freezing(current)) {
+               frozen_process(current);
+               task_unlock(current);
+       } else {
+               task_unlock(current);
+               return;
+       }
        save = current->state;
        pr_debug("%s entered refrigerator\n", current->comm);
 
-       frozen_process(current);
        spin_lock_irq(&current->sighand->siglock);
        recalc_sigpending(); /* We sent fake signal, clean it up */
        spin_unlock_irq(&current->sighand->siglock);
@@ -152,10 +160,12 @@ static unsigned int try_to_freeze_tasks(int freeze_user_space)
                        if (is_user_space(p) == !freeze_user_space)
                                continue;
 
+                       task_lock(p);
                        if (freezeable(p) && !frozen(p))
                                printk(KERN_ERR " %s\n", p->comm);
 
                        cancel_freezing(p);
+                       task_unlock(p);
                } while_each_thread(g, p);
                read_unlock(&tasklist_lock);
        }