signal: sigprocmask() should do retarget_shared_pending() (e6fa16ab) · Commits · 戴 / test

In short, almost every changing of current->blocked is wrong, or at least can lead to the unexpected results. For example. Two threads T1 and T2, T1 sleeps in sigtimedwait/pause/etc. kill(tgid, SIG) can pick T2 for TIF_SIGPENDING. If T2 calls sigprocmask() and blocks SIG before it notices the pending signal, nobody else can handle this pending shared signal. I am not sure this is bug, but at least this looks strange imho. T1 should not sleep forever, there is a signal which should wake it up. This patch moves the code which actually changes ->blocked into the new helper, set_current_blocked() and changes this code to call retarget_shared_pending() as exit_signals() does. We should only care about the signals we just blocked, we use "newset & ~current->blocked" as a mask. We do not check !sigisemptyset(newblocked), retarget_shared_pending() is cheap unless mask & shared_pending. Note: for this particular case we could simply change sigprocmask() to return -EINTR if signal_pending(), but then we should change other callers and, more importantly, if we need this fix then set_current_blocked() will have more callers and some of them can't restart. See the next patch as a random example. Signed-off-by:

Oleg Nesterov <oleg@redhat.com> Reviewed-by:

Matt Fleming <matt.fleming@linux.intel.com> Acked-by:

Tejun Heo <tj@kernel.org>

include/linux/signal.h

+1 −0

Original line number	Diff line number	Diff line
		@@ -243,6 +243,7 @@ extern long do_rt_tgsigqueueinfo(pid_t tgid, pid_t pid, int sig,
		siginfo_t *info);
		extern long do_sigpending(void __user *, unsigned long);
		extern int sigprocmask(int, sigset_t , sigset_t );
		extern void set_current_blocked(const sigset_t *);
		extern int show_unhandled_signals;

		struct pt_regs;

kernel/signal.c

+24 −5

Original line number	Diff line number	Diff line
		@@ -2299,6 +2299,29 @@ long do_no_restart_syscall(struct restart_block *param)
		return -EINTR;
		}

		/**
		* set_current_blocked - change current->blocked mask
		* @newset: new mask
		*
		* It is wrong to change ->blocked directly, this helper should be used
		* to ensure the process can't miss a shared signal we are going to block.
		*/
		void set_current_blocked(const sigset_t *newset)
		{
		struct task_struct *tsk = current;

		spin_lock_irq(&tsk->sighand->siglock);
		if (signal_pending(tsk) && !thread_group_empty(tsk)) {
		sigset_t newblocked;
		/* A set of now blocked but previously unblocked signals. */
		signandsets(&newblocked, newset, &current->blocked);
		retarget_shared_pending(tsk, &newblocked);
		}
		tsk->blocked = *newset;
		recalc_sigpending();
		spin_unlock_irq(&tsk->sighand->siglock);
		}

		/*
		* This is also useful for kernel threads that want to temporarily
		* (or permanently) block certain signals.
		@@ -2330,11 +2353,7 @@ int sigprocmask(int how, sigset_t set, sigset_t oldset)
		return -EINVAL;
		}

		spin_lock_irq(&tsk->sighand->siglock);
		tsk->blocked = newset;
		recalc_sigpending();
		spin_unlock_irq(&tsk->sighand->siglock);

		set_current_blocked(&newset);
		return 0;
		}

Admin message