Re: signalfd API issues (was Re: [PATCH/RFC] signal races/bugs, losing TIF_SIGPENDING and other woes)

[Benjamin Herrenschmidt <benh@kernel.crashing.org>]

On Tue, 2007-06-05 at 20:37 -0700, Linus Torvalds wrote:

> I agree that it would be a limitation, but it would be a sane one.
> 
> How about we try to live with that limitation, if only to avoid the issue 
> of having the private signals being stolen by anybody else. If we actually 
> find a real-live use-case where that is bad in the future, we can re-visit 
> the issue - it's always easier to _expand_ semantics later than it is to 
> restrict them, so I think this thread is a good argument for starting it 
> out in a more restricted form before people start depending on semantics 
> that can be nasty..

Here's a patch. Let me know if I missed something.

Fix races with signalfd and TIF_SIGPENDING

We must never clear TIF_SIGPENDING for another task. This patch
ensures that by preventing recalc_sigpending_tsk() from clearing
that bit if the target task is not current.

In addition we also prevent __dequeue_signal() from calling the
DRM notifier thingy when stealing signals from another task via
signalfd.

Finally, we only dequeue shared signals when called from another
task (via signalfd), we leave private signals alone.

Signed-off-by: Benjamin Herrenschmidt <benh@kernel.crashing.org>
---

Index: linux-work/kernel/signal.c
===================================================================
--- linux-work.orig/kernel/signal.c	2007-06-06 16:38:05.000000000 +1000
+++ linux-work/kernel/signal.c	2007-06-06 16:42:43.000000000 +1000
@@ -105,7 +105,12 @@ static int recalc_sigpending_tsk(struct 
 		set_tsk_thread_flag(t, TIF_SIGPENDING);
 		return 1;
 	}
-	clear_tsk_thread_flag(t, TIF_SIGPENDING);
+	/* Only clear the flag when this is issued by the target task to
+	 * clearing TIF_SIGPENDING after the target task decided to return
+	 * -ERESTARTSYS from a syscall
+	 */
+	if (t == current)
+		clear_tsk_thread_flag(t, TIF_SIGPENDING);
 	return 0;
 }
 
@@ -328,12 +333,12 @@ static int collect_signal(int sig, struc
 }
 
 static int __dequeue_signal(struct sigpending *pending, sigset_t *mask,
-			siginfo_t *info)
+			    siginfo_t *info, int stealing)
 {
 	int sig = next_signal(pending, mask);
 
 	if (sig) {
-		if (current->notifier) {
+		if (current->notifier && !stealing) {
 			if (sigismember(current->notifier_mask, sig)) {
 				if (!(current->notifier)(current->notifier_data)) {
 					clear_thread_flag(TIF_SIGPENDING);
@@ -357,10 +362,19 @@ static int __dequeue_signal(struct sigpe
  */
 int dequeue_signal(struct task_struct *tsk, sigset_t *mask, siginfo_t *info)
 {
-	int signr = __dequeue_signal(&tsk->pending, mask, info);
+	int stealing = tsk != current;
+	int signr = 0;
+
+	/* Only dequeue private signals if we are the owner, not when signals
+	 * are being stolen by another task via signalfd
+	 */
+	if (!stealing)
+		signr = __dequeue_signal(&tsk->pending, mask, info, 0);
+
+	/* No private signal, look for shared ones */
 	if (!signr) {
 		signr = __dequeue_signal(&tsk->signal->shared_pending,
-					 mask, info);
+					 mask, info, stealing);
 		/*
 		 * itimer signal ?
 		 *


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/