[patch] exclude sync signals from signalfd sets

[patch] exclude sync signals from signalfd sets

[Davide Libenzi]

The following patch excludes synchronous signals from being dequeued
by a signalfd. It also re-enable signalfd from fetching signals (that
are not synchronous) sent with sys_tkill() and sys_tgkill(), to the
thread that created the signalfd.
The behaviour of a signalfd will hence be:

- Does not, in any case, fetch synchronous signals

- Does fetch shared signals of the thread group of the thread that
  created the signalfd

- Does fetch private signals sent using sys_tkill() or sys_tgkill(),
  to the thread that created the signalfd



Signed-off-by: Davide Libenzi <davidel@xmailserver.org>


- 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);

Re: [patch] exclude sync signals from signalfd sets

[Linus Torvalds]

On Wed, 20 Jun 2007, Davide Libenzi wrote:
> 
> The following patch excludes synchronous signals from being dequeued
> by a signalfd.

I really prefer the current code. Listign special cases is just ugly. Just 
make it so that thread-local signals stay thread-local, and it's all good.

		Linus

Re: [patch] exclude sync signals from signalfd sets

[Davide Libenzi]

On Wed, 20 Jun 2007, Linus Torvalds wrote:

> 
> 
> On Wed, 20 Jun 2007, Davide Libenzi wrote:
> > 
> > The following patch excludes synchronous signals from being dequeued
> > by a signalfd.
> 
> I really prefer the current code. Listign special cases is just ugly. Just 
> make it so that thread-local signals stay thread-local, and it's all good.

Hmm, I thought the behaviour implemented by the patch was pretty clean. 
But anyway, if we leave the current code (as it is in your current tree), 
we need the patch below to avoid spurious poll returns.



- Davide


---
 fs/signalfd.c |    3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Index: linux-2.6.mod/fs/signalfd.c
===================================================================
--- linux-2.6.mod.orig/fs/signalfd.c	2007-06-20 13:08:25.000000000 -0700
+++ linux-2.6.mod/fs/signalfd.c	2007-06-20 13:09:00.000000000 -0700
@@ -133,7 +133,8 @@
 	 * the peer disconnects.
 	 */
 	if (signalfd_lock(ctx, &lk)) {
-		if (next_signal(&lk.tsk->pending, &ctx->sigmask) > 0 ||
+		if ((lk.tsk == current &&
+		     next_signal(&lk.tsk->pending, &ctx->sigmask) > 0) ||
 		    next_signal(&lk.tsk->signal->shared_pending,
 				&ctx->sigmask) > 0)
 			events |= POLLIN;

Re: [patch] exclude sync signals from signalfd sets

[Oleg Nesterov]

On 06/20, Linus Torvalds wrote:
> 
> 
> On Wed, 20 Jun 2007, Davide Libenzi wrote:
> > 
> > The following patch excludes synchronous signals from being dequeued
> > by a signalfd.
> 
> I really prefer the current code. Listign special cases is just ugly. Just 
> make it so that thread-local signals stay thread-local, and it's all good.

OK. How about the patch below then? I think this is what Nicholas suggested.

(Sorry for persistance! I can't explain why I think the current behaviour is
 not good. Just a personal feeling).

Oleg.

--- signalfd.c~	2007-06-21 13:00:21.000000000 +0400
+++ signalfd.c	2007-06-21 13:04:45.000000000 +0400
@@ -208,6 +208,15 @@ static int signalfd_copyinfo(struct sign
 	return err ? -EFAULT: sizeof(*uinfo);
 }
 
+static int
+__signalfd_dequeue(struct task_struct *tsk, sigset_t *mask, siginfo_t *info)
+{
+	if (tsk->tgid == current->tgid)
+		tsk = current;
+
+	return dequeue_signal(tsk, mask, info);
+}
+
 static ssize_t signalfd_dequeue(struct signalfd_ctx *ctx, siginfo_t *info,
 				int nonblock)
 {
@@ -218,7 +227,7 @@ static ssize_t signalfd_dequeue(struct s
 	if (!signalfd_lock(ctx, &lk))
 		return 0;
 
-	ret = dequeue_signal(lk.tsk, &ctx->sigmask, info);
+	ret = __signalfd_dequeue(lk.tsk, &ctx->sigmask, info);
 	switch (ret) {
 	case 0:
 		if (!nonblock)
@@ -232,7 +241,7 @@ static ssize_t signalfd_dequeue(struct s
 	add_wait_queue(&ctx->wqh, &wait);
 	for (;;) {
 		set_current_state(TASK_INTERRUPTIBLE);
-		ret = dequeue_signal(lk.tsk, &ctx->sigmask, info);
+		ret = __signalfd_dequeue(lk.tsk, &ctx->sigmask, info);
 		signalfd_unlock(&lk);
 		if (ret != 0)
 			break;
@@ -330,7 +339,7 @@ asmlinkage long sys_signalfd(int ufd, si
 
 		init_waitqueue_head(&ctx->wqh);
 		ctx->sigmask = sigmask;
-		ctx->tsk = current;
+		ctx->tsk = current->group_leader;
 
 		sighand = current->sighand;
 		/*