Re: [PATCH/RFC] signal races/bugs, losing TIF_SIGPENDING and otherwoes

[Linus Torvalds]

On Tue, 5 Jun 2007, Benjamin Herrenschmidt wrote:
> 
>  - something calls recalc_sigpending_tsk() on thread A (for example,
>    something try to sends it S2 which is blocked). There is no longer
>    an active signal and thus TIF_SIGPENDING is cleared on thread A

I agree. That's unquestionably a bug. We should *never* clear sigpending 
for somebody else.

I have to say, your patch looks pretty ugly though. It also looks like 
it's rife to be subtly buggy (ie somebody calls "recalc_sigpending_tsk()" 
with "current" and doesn't realize the subtle rule.

So I'd rather just make it more explicit, and simple, and just have 
something really simple like the appended instead..

If we want to be fancier (and avoid the unnecessary compare for the cases 
where we statically know that we're calling it with "t" being "current"), 
we could make it an inline function and factor things out a bit, but I'm 
not sure how worthwhile that really is.

I also wonder if we should just remove the "clear_tsk_thread_flag()" thing 
*entirely*, and do it only in the do_sigal() path. THAT, however, is a 
much bigger change. This one-liner seems the trivial and most obvious 
patch.

Comments?

		Linus

---
 kernel/signal.c |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/kernel/signal.c b/kernel/signal.c
index acdfc05..61abd8d 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -105,7 +105,8 @@ static int recalc_sigpending_tsk(struct task_struct *t)
 		set_tsk_thread_flag(t, TIF_SIGPENDING);
 		return 1;
 	}
-	clear_tsk_thread_flag(t, TIF_SIGPENDING);
+	if (t == current)
+		clear_tsk_thread_flag(t, TIF_SIGPENDING);
 	return 0;
 }
 
-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/