All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Rafael J. Wysocki" <rjw@sisk.pl>
To: Andrew Morton <akpm@linux-foundation.org>
Cc: Gautham R Shenoy <ego@in.ibm.com>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	LKML <linux-kernel@vger.kernel.org>,
	Oleg Nesterov <oleg@tv-sign.ru>, Pavel Machek <pavel@ucw.cz>,
	"Eric W. Biederman" <ebiederm@xmission.com>
Subject: [PATCH 2/7] Freezer: Close potential race between refrigerator and thaw_tasks
Date: Fri, 11 May 2007 00:37:22 +0200	[thread overview]
Message-ID: <200705110037.23268.rjw@sisk.pl> (raw)
In-Reply-To: <200705110035.32229.rjw@sisk.pl>

From: Rafael J. Wysocki <rjw@sisk.pl>

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>
---
 include/linux/freezer.h |   10 ++++++++++
 kernel/power/process.c  |   12 +++++++++++-
 2 files changed, 21 insertions(+), 1 deletion(-)

Index: linux-2.6/include/linux/freezer.h
===================================================================
--- linux-2.6.orig/include/linux/freezer.h	2007-05-10 21:44:23.000000000 +0200
+++ linux-2.6/include/linux/freezer.h	2007-05-10 21:44:31.000000000 +0200
@@ -37,14 +37,24 @@ static inline void do_not_freeze(struct 
 
 /*
  * 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;
 }
 
Index: linux-2.6/kernel/power/process.c
===================================================================
--- linux-2.6.orig/kernel/power/process.c	2007-05-10 21:44:28.000000000 +0200
+++ linux-2.6/kernel/power/process.c	2007-05-10 21:44:31.000000000 +0200
@@ -38,10 +38,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);
@@ -158,10 +166,12 @@ static unsigned int try_to_freeze_tasks(
 			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);
 	}


  parent reply	other threads:[~2007-05-11  9:07 UTC|newest]

Thread overview: 41+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-05-10 22:35 [PATCH 0/7] Freezer bugfixes Rafael J. Wysocki
2007-05-10 22:36 ` [PATCH 1/7] Freezer: Read PF_BORROWED_MM in a nonracy way Rafael J. Wysocki
2007-05-11 19:39   ` Andrew Morton
2007-05-11 20:21     ` Oleg Nesterov
2007-05-11 20:40     ` Rafael J. Wysocki
2007-05-11 22:56       ` Linus Torvalds
2007-05-11 23:20         ` Oleg Nesterov
2007-05-11 23:32           ` Linus Torvalds
2007-05-11 23:48             ` Oleg Nesterov
2007-05-12  0:05               ` Oleg Nesterov
2007-05-12  0:08               ` Linus Torvalds
2007-05-12  0:40                 ` Oleg Nesterov
2007-05-12  1:01                   ` Oleg Nesterov
2007-05-12  1:24                   ` Linus Torvalds
2007-05-12  9:01                     ` Rafael J. Wysocki
2007-05-12 10:45                       ` Rafael J. Wysocki
2007-05-12 14:18                         ` Oleg Nesterov
2007-05-12 16:35                           ` Rafael J. Wysocki
2007-05-12 16:58                             ` Oleg Nesterov
2007-05-12 17:16                               ` Rafael J. Wysocki
2007-05-12 17:43                     ` Oleg Nesterov
2007-05-12  1:11                 ` Rafael J. Wysocki
2007-05-11 23:22         ` Rafael J. Wysocki
2007-05-11 23:25           ` Andrew Morton
2007-05-12  0:16             ` Rafael J. Wysocki
2007-05-11 23:29           ` Linus Torvalds
2007-05-12  0:01             ` Rafael J. Wysocki
2007-05-12  8:16               ` Gautham R Shenoy
2007-05-12  9:27                 ` Rafael J. Wysocki
2007-05-12 10:13                   ` Gautham R Shenoy
2007-05-12 10:41                     ` Rafael J. Wysocki
2007-05-12 10:52                       ` Gautham R Shenoy
2007-05-12 11:34                         ` Rafael J. Wysocki
2007-05-12 14:25                   ` Oleg Nesterov
2007-05-12 14:59                     ` migrate_dead_tasks() vs sleep-after-exit_notify() problems? Oleg Nesterov
2007-05-10 22:37 ` Rafael J. Wysocki [this message]
2007-05-10 22:38 ` [PATCH 3/7] Freezer: Fix vfork problem Rafael J. Wysocki
2007-05-10 22:39 ` [PATCH 4/7] Freezer: Take kernel_execve into consideration Rafael J. Wysocki
2007-05-10 22:41 ` [PATCH 5/7] Freezer: Fix kthread_create vs freezer theoretical race Rafael J. Wysocki
2007-05-10 22:43 ` [PATCH 6/7] Freezer: Fix PF_NOFREEZE vs freezeable race Rafael J. Wysocki
2007-05-10 22:44 ` [PATCH 7/7] Freezer: Move frozen_process to kernel/power/process.c Rafael J. Wysocki

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=200705110037.23268.rjw@sisk.pl \
    --to=rjw@sisk.pl \
    --cc=akpm@linux-foundation.org \
    --cc=ebiederm@xmission.com \
    --cc=ego@in.ibm.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=oleg@tv-sign.ru \
    --cc=pavel@ucw.cz \
    --cc=torvalds@linux-foundation.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.