Mailing List Archive: [patch] exclude sync signals from signalfd sets

[patch] exclude sync signals from signalfd sets

Jun 20, 2007, 12:14 PM

Post #1 of 4 (302 views)

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

Re: [patch] exclude sync signals from signalfd sets [ In reply to ]

torvalds at linux-foundation

Jun 20, 2007, 12:56 PM

Post #2 of 4 (288 views)

Permalink

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
-
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/

Re: [patch] exclude sync signals from signalfd sets [ In reply to ]

davidel at xmailserver

Jun 20, 2007, 1:12 PM

Post #3 of 4 (283 views)

Permalink

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

Re: [patch] exclude sync signals from signalfd sets [ In reply to ]

oleg at tv-sign

Jun 21, 2007, 2:27 AM

Post #4 of 4 (292 views)

Permalink

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;
/*

-
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/