From 33e1c288da62a6a5aa9077a6b7bfa690b1b02cf4 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Wed, 23 May 2007 13:57:24 -0700 Subject: 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 Acked-by: Pavel Machek Cc: Gautham R Shenoy Cc: Oleg Nesterov Cc: "Eric W. Biederman" Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- kernel/power/process.c | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) (limited to 'kernel/power/process.c') diff --git a/kernel/power/process.c b/kernel/power/process.c index 088419387388..02e490e311eb 100644 --- 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(¤t->sighand->siglock); recalc_sigpending(); /* We sent fake signal, clean it up */ spin_unlock_irq(¤t->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); } -- cgit v1.2.3 From ba96a0c88098697a63e80157718b7440414ed24d Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Wed, 23 May 2007 13:57:25 -0700 Subject: freezer: fix vfork problem Currently try_to_freeze_tasks() has to wait until all of the vforked processes exit and for this reason every user can make it fail. To fix this problem we can introduce the additional process flag PF_FREEZER_SKIP to be used by tasks that do not want to be counted as freezable by the freezer and want to have TIF_FREEZE set nevertheless. Then, this flag can be set by tasks using sys_vfork() before they call wait_for_completion(&vfork) and cleared after they have woken up. After clearing it, the tasks should call try_to_freeze() as soon as possible. Signed-off-by: Rafael J. Wysocki Cc: Gautham R Shenoy Cc: Oleg Nesterov Cc: Pavel Machek Cc: "Eric W. Biederman" Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- include/linux/freezer.h | 48 ++++++++++++++++++++++++++++++++++++++++++++++-- include/linux/sched.h | 1 + kernel/fork.c | 3 +++ kernel/power/process.c | 29 +++++++++-------------------- 4 files changed, 59 insertions(+), 22 deletions(-) (limited to 'kernel/power/process.c') diff --git a/include/linux/freezer.h b/include/linux/freezer.h index db5423eae24d..c9435252e8e4 100644 --- a/include/linux/freezer.h +++ b/include/linux/freezer.h @@ -81,7 +81,49 @@ static inline int try_to_freeze(void) return 0; } -extern void thaw_some_processes(int all); +/* + * The PF_FREEZER_SKIP flag should be set by a vfork parent right before it + * calls wait_for_completion(&vfork) and reset right after it returns from this + * function. Next, the parent should call try_to_freeze() to freeze itself + * appropriately in case the child has exited before the freezing of tasks is + * complete. However, we don't want kernel threads to be frozen in unexpected + * places, so we allow them to block freeze_processes() instead or to set + * PF_NOFREEZE if needed and PF_FREEZER_SKIP is only set for userland vfork + * parents. Fortunately, in the ____call_usermodehelper() case the parent won't + * really block freeze_processes(), since ____call_usermodehelper() (the child) + * does a little before exec/exit and it can't be frozen before waking up the + * parent. + */ + +/* + * If the current task is a user space one, tell the freezer not to count it as + * freezable. + */ +static inline void freezer_do_not_count(void) +{ + if (current->mm) + current->flags |= PF_FREEZER_SKIP; +} + +/* + * If the current task is a user space one, tell the freezer to count it as + * freezable again and try to freeze it. + */ +static inline void freezer_count(void) +{ + if (current->mm) { + current->flags &= ~PF_FREEZER_SKIP; + try_to_freeze(); + } +} + +/* + * Check if the task should be counted as freezeable by the freezer + */ +static inline int freezer_should_skip(struct task_struct *p) +{ + return !!(p->flags & PF_FREEZER_SKIP); +} #else static inline int frozen(struct task_struct *p) { return 0; } @@ -96,5 +138,7 @@ static inline void thaw_processes(void) {} static inline int try_to_freeze(void) { return 0; } - +static inline void freezer_do_not_count(void) {} +static inline void freezer_count(void) {} +static inline int freezer_should_skip(struct task_struct *p) { return 0; } #endif diff --git a/include/linux/sched.h b/include/linux/sched.h index a81897e2a244..870b75e348ab 100644 --- a/include/linux/sched.h +++ b/include/linux/sched.h @@ -1182,6 +1182,7 @@ static inline void put_task_struct(struct task_struct *t) #define PF_SPREAD_SLAB 0x02000000 /* Spread some slab caches over cpuset */ #define PF_MEMPOLICY 0x10000000 /* Non-default NUMA mempolicy */ #define PF_MUTEX_TESTER 0x20000000 /* Thread belongs to the rt mutex tester */ +#define PF_FREEZER_SKIP 0x40000000 /* Freezer should not count it as freezeable */ /* * Only the _current_ task can read/write to tsk->flags, but other diff --git a/kernel/fork.c b/kernel/fork.c index 87069cfc18a1..73ad5cda1bcd 100644 --- a/kernel/fork.c +++ b/kernel/fork.c @@ -45,6 +45,7 @@ #include #include #include +#include #include #include #include @@ -1405,7 +1406,9 @@ long do_fork(unsigned long clone_flags, } if (clone_flags & CLONE_VFORK) { + freezer_do_not_count(); wait_for_completion(&vfork); + freezer_count(); if (unlikely (current->ptrace & PT_TRACE_VFORK_DONE)) { current->ptrace_message = nr; ptrace_notify ((PTRACE_EVENT_VFORK_DONE << 8) | SIGTRAP); diff --git a/kernel/power/process.c b/kernel/power/process.c index 02e490e311eb..eefca8581fa0 100644 --- a/kernel/power/process.c +++ b/kernel/power/process.c @@ -120,22 +120,12 @@ static unsigned int try_to_freeze_tasks(int freeze_user_space) cancel_freezing(p); continue; } - if (is_user_space(p)) { - if (!freeze_user_space) - continue; - - /* Freeze the task unless there is a vfork - * completion pending - */ - if (!p->vfork_done) - freeze_process(p); - } else { - if (freeze_user_space) - continue; - - freeze_process(p); - } - todo++; + if (is_user_space(p) == !freeze_user_space) + continue; + + freeze_process(p); + if (!freezer_should_skip(p)) + todo++; } while_each_thread(g, p); read_unlock(&tasklist_lock); yield(); /* Yield is okay here */ @@ -161,7 +151,8 @@ static unsigned int try_to_freeze_tasks(int freeze_user_space) continue; task_lock(p); - if (freezeable(p) && !frozen(p)) + if (freezeable(p) && !frozen(p) && + !freezer_should_skip(p)) printk(KERN_ERR " %s\n", p->comm); cancel_freezing(p); @@ -210,9 +201,7 @@ static void thaw_tasks(int thaw_user_space) if (is_user_space(p) == !thaw_user_space) continue; - if (!thaw_process(p)) - printk(KERN_WARNING " Strange, %s not stopped\n", - p->comm ); + thaw_process(p); } while_each_thread(g, p); read_unlock(&tasklist_lock); } -- cgit v1.2.3 From 49b12d4f5e274517b8bc032d507abf31cc2f4150 Mon Sep 17 00:00:00 2001 From: "Rafael J. Wysocki" Date: Wed, 23 May 2007 13:57:26 -0700 Subject: freezer: take kernel_execve into consideration Kernel threads can become userland processes by calling kernel_execve(). In particular, this may happen right after the try_to_freeze_tasks() called with FREEZER_USER_SPACE has returned, so try_to_freeze_tasks() needs to take userspace processes into consideration even if it is called with FREEZER_KERNEL_THREADS. Signed-off-by: Rafael J. Wysocki Acked-by: Pavel Machek Cc: Gautham R Shenoy Cc: Oleg Nesterov Cc: "Eric W. Biederman" Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- kernel/power/process.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) (limited to 'kernel/power/process.c') diff --git a/kernel/power/process.c b/kernel/power/process.c index eefca8581fa0..2cea2658e985 100644 --- a/kernel/power/process.c +++ b/kernel/power/process.c @@ -120,7 +120,7 @@ static unsigned int try_to_freeze_tasks(int freeze_user_space) cancel_freezing(p); continue; } - if (is_user_space(p) == !freeze_user_space) + if (freeze_user_space && !is_user_space(p)) continue; freeze_process(p); @@ -147,7 +147,7 @@ static unsigned int try_to_freeze_tasks(int freeze_user_space) TIMEOUT / HZ, todo); read_lock(&tasklist_lock); do_each_thread(g, p) { - if (is_user_space(p) == !freeze_user_space) + if (freeze_user_space && !is_user_space(p)) continue; task_lock(p); -- cgit v1.2.3 From 88f18ba028b5939bb6f77bd690e5ad8d01bb24cc Mon Sep 17 00:00:00 2001 From: Gautham R Shenoy Date: Wed, 23 May 2007 13:57:29 -0700 Subject: freezer: move frozen_process() to kernel/power/process.c Other than refrigerator, no one else calls frozen_process(). So move it from include/linux/freezer.h to kernel/power/process.c. Also, since a task can be marked as frozen by itself, we don't need to pass the (struct task_struct *p) parameter to frozen_process(). Signed-off-by: Gautham R Shenoy Signed-off-by: Rafael J. Wysocki Cc: Oleg Nesterov Cc: Pavel Machek Cc: "Eric W. Biederman" Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- include/linux/freezer.h | 13 ------------- kernel/power/process.c | 14 +++++++++++++- 2 files changed, 13 insertions(+), 14 deletions(-) (limited to 'kernel/power/process.c') diff --git a/include/linux/freezer.h b/include/linux/freezer.h index 1045ee9c0bb4..4631086f5060 100644 --- a/include/linux/freezer.h +++ b/include/linux/freezer.h @@ -58,18 +58,6 @@ static inline int thaw_process(struct task_struct *p) return 0; } -/* - * freezing is complete, mark process as frozen - */ -static inline void frozen_process(struct task_struct *p) -{ - if (!unlikely(p->flags & PF_NOFREEZE)) { - p->flags |= PF_FROZEN; - wmb(); - } - clear_tsk_thread_flag(p, TIF_FREEZE); -} - extern void refrigerator(void); extern int freeze_processes(void); extern void thaw_processes(void); @@ -132,7 +120,6 @@ static inline int frozen(struct task_struct *p) { return 0; } static inline int freezing(struct task_struct *p) { return 0; } static inline void freeze(struct task_struct *p) { BUG(); } static inline int thaw_process(struct task_struct *p) { return 1; } -static inline void frozen_process(struct task_struct *p) { BUG(); } static inline void refrigerator(void) {} static inline int freeze_processes(void) { BUG(); return 0; } diff --git a/kernel/power/process.c b/kernel/power/process.c index 2cea2658e985..d31d638ab4c0 100644 --- a/kernel/power/process.c +++ b/kernel/power/process.c @@ -31,6 +31,18 @@ static inline int freezeable(struct task_struct * p) return 1; } +/* + * freezing is complete, mark current process as frozen + */ +static inline void frozen_process(void) +{ + if (!unlikely(current->flags & PF_NOFREEZE)) { + current->flags |= PF_FROZEN; + wmb(); + } + clear_tsk_thread_flag(current, TIF_FREEZE); +} + /* Refrigerator is place where frozen processes are stored :-). */ void refrigerator(void) { @@ -40,7 +52,7 @@ void refrigerator(void) task_lock(current); if (freezing(current)) { - frozen_process(current); + frozen_process(); task_unlock(current); } else { task_unlock(current); -- cgit v1.2.3 From 7bb44adef39ad3bda2be40bb34686bc56bd563a5 Mon Sep 17 00:00:00 2001 From: Roland McGrath Date: Wed, 23 May 2007 13:57:44 -0700 Subject: recalc_sigpending_tsk fixes Steve Hawkes discovered a problem where recalc_sigpending_tsk was called in do_sigaction but no signal_wake_up call was made, preventing later signals from waking up blocked threads with TIF_SIGPENDING already set. In fact, the few other calls to recalc_sigpending_tsk outside the signals code are also subject to this problem in other race conditions. This change makes recalc_sigpending_tsk private to the signals code. It changes the outside calls, as well as do_sigaction, to use the new recalc_sigpending_and_wake instead. Signed-off-by: Roland McGrath Cc: Cc: Oleg Nesterov Signed-off-by: Andrew Morton Signed-off-by: Linus Torvalds --- include/linux/sched.h | 12 +++++++----- kernel/exit.c | 7 ++----- kernel/power/process.c | 2 +- kernel/signal.c | 24 ++++++++++++++++++------ 4 files changed, 28 insertions(+), 17 deletions(-) (limited to 'kernel/power/process.c') diff --git a/include/linux/sched.h b/include/linux/sched.h index 870b75e348ab..d58e74b98367 100644 --- a/include/linux/sched.h +++ b/include/linux/sched.h @@ -1616,11 +1616,13 @@ static inline int lock_need_resched(spinlock_t *lock) return 0; } -/* Reevaluate whether the task has signals pending delivery. - This is required every time the blocked sigset_t changes. - callers must hold sighand->siglock. */ - -extern FASTCALL(void recalc_sigpending_tsk(struct task_struct *t)); +/* + * Reevaluate whether the task has signals pending delivery. + * Wake the task if so. + * This is required every time the blocked sigset_t changes. + * callers must hold sighand->siglock. + */ +extern void recalc_sigpending_and_wake(struct task_struct *t); extern void recalc_sigpending(void); extern void signal_wake_up(struct task_struct *t, int resume_stopped); diff --git a/kernel/exit.c b/kernel/exit.c index c6d14b8008dd..5b888c24e43e 100644 --- a/kernel/exit.c +++ b/kernel/exit.c @@ -762,11 +762,8 @@ static void exit_notify(struct task_struct *tsk) read_lock(&tasklist_lock); spin_lock_irq(&tsk->sighand->siglock); for (t = next_thread(tsk); t != tsk; t = next_thread(t)) - if (!signal_pending(t) && !(t->flags & PF_EXITING)) { - recalc_sigpending_tsk(t); - if (signal_pending(t)) - signal_wake_up(t, 0); - } + if (!signal_pending(t) && !(t->flags & PF_EXITING)) + recalc_sigpending_and_wake(t); spin_unlock_irq(&tsk->sighand->siglock); read_unlock(&tasklist_lock); } diff --git a/kernel/power/process.c b/kernel/power/process.c index d31d638ab4c0..e0233d8422b9 100644 --- a/kernel/power/process.c +++ b/kernel/power/process.c @@ -101,7 +101,7 @@ static void cancel_freezing(struct task_struct *p) pr_debug(" clean up: %s\n", p->comm); do_not_freeze(p); spin_lock_irqsave(&p->sighand->siglock, flags); - recalc_sigpending_tsk(p); + recalc_sigpending_and_wake(p); spin_unlock_irqrestore(&p->sighand->siglock, flags); } } diff --git a/kernel/signal.c b/kernel/signal.c index 364fc95bf97c..acdfc0549c6f 100644 --- a/kernel/signal.c +++ b/kernel/signal.c @@ -96,15 +96,27 @@ static inline int has_pending_signals(sigset_t *signal, sigset_t *blocked) #define PENDING(p,b) has_pending_signals(&(p)->signal, (b)) -fastcall void recalc_sigpending_tsk(struct task_struct *t) +static int recalc_sigpending_tsk(struct task_struct *t) { if (t->signal->group_stop_count > 0 || (freezing(t)) || PENDING(&t->pending, &t->blocked) || - PENDING(&t->signal->shared_pending, &t->blocked)) + PENDING(&t->signal->shared_pending, &t->blocked)) { set_tsk_thread_flag(t, TIF_SIGPENDING); - else - clear_tsk_thread_flag(t, TIF_SIGPENDING); + return 1; + } + clear_tsk_thread_flag(t, TIF_SIGPENDING); + return 0; +} + +/* + * After recalculating TIF_SIGPENDING, we need to make sure the task wakes up. + * This is superfluous when called on current, the wakeup is a harmless no-op. + */ +void recalc_sigpending_and_wake(struct task_struct *t) +{ + if (recalc_sigpending_tsk(t)) + signal_wake_up(t, 0); } void recalc_sigpending(void) @@ -744,7 +756,7 @@ force_sig_info(int sig, struct siginfo *info, struct task_struct *t) action->sa.sa_handler = SIG_DFL; if (blocked) { sigdelset(&t->blocked, sig); - recalc_sigpending_tsk(t); + recalc_sigpending_and_wake(t); } } ret = specific_send_sig_info(sig, info, t); @@ -2273,7 +2285,7 @@ int do_sigaction(int sig, struct k_sigaction *act, struct k_sigaction *oact) rm_from_queue_full(&mask, &t->signal->shared_pending); do { rm_from_queue_full(&mask, &t->pending); - recalc_sigpending_tsk(t); + recalc_sigpending_and_wake(t); t = next_thread(t); } while (t != current); } -- cgit v1.2.3