nagai/kernel
3
0
Fork 0
forked from mirrors/linux
Code Pull requests Activity

signal: sigprocmask() should do retarget_shared_pending()

Browse source 
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>
This commit is contained in:
Oleg Nesterov 2011-04-27 20:59:41 +02:00
parent 73ef4aeb61
commit e6fa16ab9c
2 changed files with 25 additions and 5 deletions
repo.diff.show_diff_stats Download patch file Download diff file Expand all files Collapse all files

1
include/linux/signal.h
View file

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

29
kernel/signal.c
View file

@ -2299,6 +2299,29 @@ long do_no_restart_syscall(struct restart_block *param)
return -EINTR; 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 * This is also useful for kernel threads that want to temporarily
* (or permanently) block certain signals. * (or permanently) block certain signals.
@ -2330,11 +2353,7 @@ int sigprocmask(int how, sigset_t *set, sigset_t *oldset)
return -EINVAL; return -EINVAL;
} }
spin_lock_irq(&tsk->sighand->siglock); set_current_blocked(&newset);
tsk->blocked = newset;
recalc_sigpending();
spin_unlock_irq(&tsk->sighand->siglock);
return 0; return 0;
} }