Commit 1deac632 authored Apr 01, 2011 by Oleg Nesterov Committed by Tejun Heo Apr 04, 2011

signal: prepare_signal(SIGCONT) shouldn't play with TIF_SIGPENDING



prepare_signal(SIGCONT) should never set TIF_SIGPENDING or wake up
the TASK_INTERRUPTIBLE threads. We are going to call complete_signal()
which should pick the right thread correctly. All we need is to wake
up the TASK_STOPPED threads.

If the task was stopped, it can't return to usermode without taking
->siglock. Otherwise we don't care, and the spurious TIF_SIGPENDING
can't be useful.

The comment says:

	* If there is a handler for SIGCONT, we must make
	* sure that no thread returns to user mode before
	* we post the signal

It is not clear what this means. Probably, "when there's only a single
thread" and this continues to be true. Otherwise, even if this SIGCONT
is not private, with or without this change only one thread can dequeue
SIGCONT, other threads can happily return to user mode before before
that thread handles this signal.

Note also that wake_up_state(t, __TASK_STOPPED) can't race with the task
which changes its state, TASK_STOPPED state is protected by ->siglock as
well.

In short: when it comes to signal delivery, SIGCONT is the normal signal
and does not need any special support.

Signed-off-by: Oleg Nesterov <oleg@redhat.com>
Signed-off-by: Tejun Heo <tj@kernel.org>

parent 244056f9

kernel/signal.c

+2 −25

Original line number	Diff line number	Diff line
		@@ -788,37 +788,14 @@ static int prepare_signal(int sig, struct task_struct *p, int from_ancestor_ns)
		} else if (sig == SIGCONT) {
		unsigned int why;
		/*
		* Remove all stop signals from all queues,
		* and wake all threads.
		* Remove all stop signals from all queues, wake all threads.
		*/
		rm_from_queue(SIG_KERNEL_STOP_MASK, &signal->shared_pending);
		t = p;
		do {
		unsigned int state;

		task_clear_group_stop_pending(t);

		rm_from_queue(SIG_KERNEL_STOP_MASK, &t->pending);
		/*
		* If there is a handler for SIGCONT, we must make
		* sure that no thread returns to user mode before
		* we post the signal, in case it was the only
		* thread eligible to run the signal handler--then
		* it must not do anything between resuming and
		* running the handler. With the TIF_SIGPENDING
		* flag set, the thread will pause and acquire the
		* siglock that we hold now and until we've queued
		* the pending signal.
		*
		* Wake up the stopped thread _after_ setting
		* TIF_SIGPENDING
		*/
		state = __TASK_STOPPED;
		if (sig_user_defined(t, SIGCONT) && !sigismember(&t->blocked, SIGCONT)) {
		set_tsk_thread_flag(t, TIF_SIGPENDING);
		state \|= TASK_INTERRUPTIBLE;
		}
		wake_up_state(t, state);
		wake_up_state(t, __TASK_STOPPED);
		} while_each_thread(p, t);

		/*

Admin message