mirror of
				https://github.com/torvalds/linux.git
				synced 2025-11-04 10:40:15 +02:00 
			
		
		
		
	signal: sigprocmask() should do retarget_shared_pending()
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:
		
							parent
							
								
									73ef4aeb61
								
							
						
					
					
						commit
						e6fa16ab9c
					
				
					 2 changed files with 25 additions and 5 deletions
				
			
		| 
						 | 
				
			
			@ -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;
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
| 
						 | 
				
			
			@ -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, ¤t->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;
 | 
			
		||||
}
 | 
			
		||||
 | 
			
		||||
| 
						 | 
				
			
			
 | 
			
		|||
		Loading…
	
		Reference in a new issue