Commit 5d8f72b5 authored by Oleg Nesterov's avatar Oleg Nesterov Committed by Tejun Heo
Browse files

freezer: change ptrace_stop/do_signal_stop to use freezable_schedule() 



try_to_freeze_tasks() and cgroup_freezer rely on scheduler locks
to ensure that a task doing STOPPED/TRACED -> RUNNING transition
can't escape freezing. This mostly works, but ptrace_stop() does
not necessarily call schedule(), it can change task->state back to
RUNNING and check freezing() without any lock/barrier in between.

We could add the necessary barrier, but this patch changes
ptrace_stop() and do_signal_stop() to use freezable_schedule().
This fixes the race, freezer_count() and freezer_should_skip()
carefully avoid the race.

And this simplifies the code, try_to_freeze_tasks/update_if_frozen
no longer need to use task_is_stopped_or_traced() checks with the
non trivial assumptions. We can rely on the mechanism which was
specially designed to mark the sleeping task as "frozen enough".

v2: As Tejun pointed out, we can also change get_signal_to_deliver()
and move try_to_freeze() up before 'relock' label.

Signed-off-by: default avatarOleg Nesterov <oleg@redhat.com>
Signed-off-by: default avatarTejun Heo <tj@kernel.org>
parent ead5c473

include/linux/freezer.h

+3 −4
Original line number Diff line number Diff line
@@ -134,10 +134,9 @@ static inline bool freezer_should_skip(struct task_struct *p) 
}



/*

 * These macros are intended to be used whenever you want allow a task that's

 * sleeping in TASK_UNINTERRUPTIBLE or TASK_KILLABLE state to be frozen. Note

 * that neither return any clear indication of whether a freeze event happened

 * while in this function.

 * These macros are intended to be used whenever you want allow a sleeping

 * task to be frozen. Note that neither return any clear indication of

 * whether a freeze event happened while in this function.

 */



/* Like schedule(), but should not block the freezer. */

kernel/cgroup_freezer.c

+1 −2
Original line number Diff line number Diff line
@@ -198,8 +198,7 @@ static void update_if_frozen(struct cgroup *cgroup, struct freezer *freezer) 
			 * completion.  Consider it frozen in addition to

			 * the usual frozen condition.

			 */

			if (!frozen(task) && !task_is_stopped_or_traced(task) &&

			    !freezer_should_skip(task))

			if (!frozen(task) && !freezer_should_skip(task))

				goto notyet;

		}

	}

kernel/freezer.c

+2 −9
Original line number Diff line number Diff line
@@ -116,17 +116,10 @@ bool freeze_task(struct task_struct *p) 
		return false;

	}



	if (!(p->flags & PF_KTHREAD)) {

	if (!(p->flags & PF_KTHREAD))

		fake_signal_wake_up(p);

		/*

		 * fake_signal_wake_up() goes through p's scheduler

		 * lock and guarantees that TASK_STOPPED/TRACED ->

		 * TASK_RUNNING transition can't race with task state

		 * testing in try_to_freeze_tasks().

		 */

	} else {

	else

		wake_up_state(p, TASK_INTERRUPTIBLE);

	}



	spin_unlock_irqrestore(&freezer_lock, flags);

	return true;

kernel/power/process.c

+1 −12
Original line number Diff line number Diff line
@@ -48,18 +48,7 @@ static int try_to_freeze_tasks(bool user_only) 
			if (p == current || !freeze_task(p))

				continue;



			/*

			 * Now that we've done set_freeze_flag, don't

			 * perturb a task in TASK_STOPPED or TASK_TRACED.

			 * It is "frozen enough".  If the task does wake

			 * up, it will immediately call try_to_freeze.

			 *

			 * Because freeze_task() goes through p's scheduler lock, it's

			 * guaranteed that TASK_STOPPED/TRACED -> TASK_RUNNING

			 * transition can't race with task state testing here.

			 */

			if (!task_is_stopped_or_traced(p) &&

			    !freezer_should_skip(p))

			if (!freezer_should_skip(p))

				todo++;

		} while_each_thread(g, p);

		read_unlock(&tasklist_lock);

kernel/signal.c

+6 −14
Original line number Diff line number Diff line
@@ -1908,7 +1908,7 @@ static void ptrace_stop(int exit_code, int why, int clear_code, siginfo_t *info) 
		preempt_disable();

		read_unlock(&tasklist_lock);

		preempt_enable_no_resched();

		schedule();

		freezable_schedule();

	} else {

		/*

		 * By the time we got the lock, our tracer went away.
@@ -1929,13 +1929,6 @@ static void ptrace_stop(int exit_code, int why, int clear_code, siginfo_t *info) 
		read_unlock(&tasklist_lock);

	}



	/*

	 * While in TASK_TRACED, we were considered "frozen enough".

	 * Now that we woke up, it's crucial if we're supposed to be

	 * frozen that we freeze now before running anything substantial.

	 */

	try_to_freeze();



	/*

	 * We are back.  Now reacquire the siglock before touching

	 * last_siginfo, so that we are sure to have synchronized with
@@ -2092,7 +2085,7 @@ static bool do_signal_stop(int signr) 
		}



		/* Now we don't run again until woken by SIGCONT or SIGKILL */

		schedule();

		freezable_schedule();

		return true;

	} else {

		/*
@@ -2200,15 +2193,14 @@ int get_signal_to_deliver(siginfo_t *info, struct k_sigaction *return_ka, 
	if (unlikely(uprobe_deny_signal()))

		return 0;



relock:

	/*

	 * We'll jump back here after any time we were stopped in TASK_STOPPED.

	 * While in TASK_STOPPED, we were considered "frozen enough".

	 * Now that we woke up, it's crucial if we're supposed to be

	 * frozen that we freeze now before running anything substantial.

	 * Do this once, we can't return to user-mode if freezing() == T.

	 * do_signal_stop() and ptrace_stop() do freezable_schedule() and

	 * thus do not need another check after return.

	 */

	try_to_freeze();



relock:

	spin_lock_irq(&sighand->siglock);

	/*

	 * Every stopped thread goes here after wakeup. Check to see if

CRA Git | Maintained and supported by SUSTech CRA and CCSE