Re: Fix signalfd interaction with thread-private signals

[Davide Libenzi]

On Wed, 20 Jun 2007, Benjamin Herrenschmidt wrote:

> On Tue, 2007-06-19 at 16:49 -0700, Davide Libenzi wrote:
> 
> > Actually, I think signalfd is fine as is, with Ben's patch applied. 
> > Signalfd should only fetch shared signals, not specific ones (in any 
> > case).
> 
> The only advantage of that additional patch is that it will allow any
> thread to fetch its own private signals via signalfd, regardless of what
> context is attached to the signalfd in the first place.
> 
> Without the patch (with what's currently in Linus tree), a thread will
> only get its own private signals if it's also the thread that created
> the signalfd (and thus is attached to the signalfd "context").
> 
> I don't mind either way, up to you guys to decide what semantics you
> want.

Ok, why instead don't we go for something like the attached patch?
We exclude sync signals from signalfd, but we don't limit signalfd to 
shared signals. Ie, we should be able to fetch a signal sent with 
sys_tkill() to threads different from "current", that otherwise we would 
not be able to fetch.
Ben, sorry but my memory sucks ... the "notifier" thing was fine in that 
case, no?




- Davide



---
 fs/signalfd.c   |   10 +++++++++-
 kernel/signal.c |    6 +-----
 2 files changed, 10 insertions(+), 6 deletions(-)

Index: linux-2.6.mod/fs/signalfd.c
===================================================================
--- linux-2.6.mod.orig/fs/signalfd.c	2007-06-19 18:58:15.000000000 -0700
+++ linux-2.6.mod/fs/signalfd.c	2007-06-19 19:02:57.000000000 -0700
@@ -26,6 +26,14 @@
 #include <linux/anon_inodes.h>
 #include <linux/signalfd.h>
 
+/*
+ * Signals that a signalfd should never fetch.
+ */
+#define SIGNALFD_EXCLUDE_MASK (sigmask(SIGILL) | sigmask(SIGKILL) | \
+				       sigmask(SIGSTOP) | sigmask(SIGTRAP) | \
+				       sigmask(SIGFPE) | sigmask(SIGSEGV) | \
+				       sigmask(SIGBUS))
+
 struct signalfd_ctx {
 	struct list_head lnk;
 	wait_queue_head_t wqh;
@@ -320,7 +328,7 @@
 	if (sizemask != sizeof(sigset_t) ||
 	    copy_from_user(&sigmask, user_mask, sizeof(sigmask)))
 		return error = -EINVAL;
-	sigdelsetmask(&sigmask, sigmask(SIGKILL) | sigmask(SIGSTOP));
+	sigdelsetmask(&sigmask, SIGNALFD_EXCLUDE_MASK);
 	signotset(&sigmask);
 
 	if (ufd == -1) {
Index: linux-2.6.mod/kernel/signal.c
===================================================================
--- linux-2.6.mod.orig/kernel/signal.c	2007-06-19 18:55:07.000000000 -0700
+++ linux-2.6.mod/kernel/signal.c	2007-06-19 18:58:43.000000000 -0700
@@ -365,11 +365,7 @@
 {
 	int signr = 0;
 
-	/* We only dequeue private signals from ourselves, we don't let
-	 * signalfd steal them
-	 */
-	if (tsk == current)
-		signr = __dequeue_signal(&tsk->pending, mask, info);
+	signr = __dequeue_signal(&tsk->pending, mask, info);
 	if (!signr) {
 		signr = __dequeue_signal(&tsk->signal->shared_pending,
 					 mask, info);
-
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/