All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 2.6.16-mm1] send_sigqueue: simplify and fix the race
@ 2006-03-26  2:47 Oleg Nesterov
  0 siblings, 0 replies; only message in thread
From: Oleg Nesterov @ 2006-03-26  2:47 UTC (permalink / raw)
  To: Andrew Morton
  Cc: linux-kernel, Paul E. McKenney, Thomas Gleixner, Roman Zippel

send_sigqueue() checks PF_EXITING, then locks p->sighand->siglock.
This is unsafe: 'p' can exit in between and set ->sighand = NULL.
The race is theoretical, the window is tiny and irqs are disabled
by the caller, so I don't think we need the fix for -stable tree.

Convert send_sigqueue() to use lock_task_sighand() helper.

Also, delete 'p->flags & PF_EXITING' re-check, it is unneeded and
the comment is wrong.

Signed-off-by: Oleg Nesterov <oleg@tv-sign.ru>

--- MM/kernel/signal.c~5_SSQ	2006-03-24 21:24:33.000000000 +0300
+++ MM/kernel/signal.c	2006-03-25 20:18:38.000000000 +0300
@@ -1309,12 +1309,10 @@ void sigqueue_free(struct sigqueue *q)
 	__sigqueue_free(q);
 }
 
-int
-send_sigqueue(int sig, struct sigqueue *q, struct task_struct *p)
+int send_sigqueue(int sig, struct sigqueue *q, struct task_struct *p)
 {
 	unsigned long flags;
 	int ret = 0;
-	struct sighand_struct *sh;
 
 	BUG_ON(!(q->flags & SIGQUEUE_PREALLOC));
 
@@ -1328,48 +1326,17 @@ send_sigqueue(int sig, struct sigqueue *
 	 */
 	rcu_read_lock();
 
-	if (unlikely(p->flags & PF_EXITING)) {
+	if (!likely(lock_task_sighand(p, &flags))) {
 		ret = -1;
 		goto out_err;
 	}
 
-retry:
-	sh = rcu_dereference(p->sighand);
-
-	spin_lock_irqsave(&sh->siglock, flags);
-	if (p->sighand != sh) {
-		/* We raced with exec() in a multithreaded process... */
-		spin_unlock_irqrestore(&sh->siglock, flags);
-		goto retry;
-	}
-
-	/*
-	 * We do the check here again to handle the following scenario:
-	 *
-	 * CPU 0		CPU 1
-	 * send_sigqueue
-	 * check PF_EXITING
-	 * interrupt		exit code running
-	 *			__exit_signal
-	 *			lock sighand->siglock
-	 *			unlock sighand->siglock
-	 * lock sh->siglock
-	 * add(tsk->pending) 	flush_sigqueue(tsk->pending)
-	 *
-	 */
-
-	if (unlikely(p->flags & PF_EXITING)) {
-		ret = -1;
-		goto out;
-	}
-
 	if (unlikely(!list_empty(&q->list))) {
 		/*
 		 * If an SI_TIMER entry is already queue just increment
 		 * the overrun count.
 		 */
-		if (q->info.si_code != SI_TIMER)
-			BUG();
+		BUG_ON(q->info.si_code != SI_TIMER);
 		q->info.si_overrun++;
 		goto out;
 	}
@@ -1385,7 +1352,7 @@ retry:
 		signal_wake_up(p, sig == SIGKILL);
 
 out:
-	spin_unlock_irqrestore(&sh->siglock, flags);
+	unlock_task_sighand(p, &flags);
 out_err:
 	rcu_read_unlock();
 


^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2006-03-25 23:50 UTC | newest]

Thread overview: (only message) (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-03-26  2:47 [PATCH 2.6.16-mm1] send_sigqueue: simplify and fix the race Oleg Nesterov

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.