linux-kernel - Re: Fix signalfd interaction with thread-private signals

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:	Tue, 19 Jun 2007 19:15:50 -0700 (PDT)
From:	Davide Libenzi <davidel@...ilserver.org>
To:	Benjamin Herrenschmidt <benh@...nel.crashing.org>
cc:	Oleg Nesterov <oleg@...sign.ru>,
	Linus Torvalds <torvalds@...ux-foundation.org>,
	Nicholas Miell <nmiell@...cast.net>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: Fix signalfd interaction with thread-private signals

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@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/