linux-kernel - Re: synchronous signal in the blocked signal context

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, 1 Aug 2006 08:25:12 -0700 (PDT)
From:	Linus Torvalds <torvalds@...l.org>
To:	"Paul E. McKenney" <paulmck@...ibm.com>
cc:	"Siddha, Suresh B" <suresh.b.siddha@...el.com>,
	Andrew Morton <akpm@...l.org>,
	Linux Kernel Mailing List <linux-kernel@...r.kernel.org>
Subject: Re: synchronous signal in the blocked signal context

On Tue, 1 Aug 2006, Paul E. McKenney wrote:
> > 
> > Paul? Should I just revert, or did you have some deeper reason for it?
> 
> I cannot claim any deep thought on this one, so please do revert it.

Well, I do have to say that I like the notion of trying to have the _same_ 
semantics for "force_sig_info()" and "force_sig_specific()", so in that 
way your patch is fine - I just missed the fact that it changed it back to 
the old broken ones (that results in endless SIGSEGV's if the SIGSEGV 
happens when setting up the handler for the SIGSEGV and other 
"interesting" issues, where a bug can result in the user process hanging 
instead of just killing it outright).

However, I wonder if the _proper_ fix is to just either remove 
"force_sig_specific()" entirely, or just make that one match the semantics 
of "force_sig_info()" instead (rather than doing it the other way - change 
for_sig_specific() to match force_sig_info()).

force_sig_info() has only two uses, and both should be ok with the 
force_sig_specific() semantics, since they are for SIGSTOP and SIGKILL 
respectively, and those should not be blockable unless you're a kernel 
thread (and I don't think either of them could validly ever be used with 
kernel threads anyway), so doing it the other way around _should_ be ok.

Paul, Suresh, would something like this work for you instead?

		Linus
----
diff --git a/kernel/signal.c b/kernel/signal.c
index 7fe874d..bfdb568 100644
--- a/kernel/signal.c
+++ b/kernel/signal.c
@@ -791,22 +791,31 @@ out:
 /*
  * Force a signal that the process can't ignore: if necessary
  * we unblock the signal and change any SIG_IGN to SIG_DFL.
+ *
+ * Note: If we unblock the signal, we always reset it to SIG_DFL,
+ * since we do not want to have a signal handler that was blocked
+ * be invoked when user space had explicitly blocked it.
+ *
+ * We don't want to have recursive SIGSEGV's etc, for example.
  */
-
 int
 force_sig_info(int sig, struct siginfo *info, struct task_struct *t)
 {
 	unsigned long int flags;
-	int ret;
+	int ret, blocked, ignored;
+	struct k_sigaction *action;

 	spin_lock_irqsave(&t->sighand->siglock, flags);
-	if (t->sighand->action[sig-1].sa.sa_handler == SIG_IGN) {
-		t->sighand->action[sig-1].sa.sa_handler = SIG_DFL;
-	}
-	if (sigismember(&t->blocked, sig)) {
-		sigdelset(&t->blocked, sig);
+	action = &t->sighand->action[sig-1];
+	ignored = action->sa.sa_handler == SIG_IGN;
+	blocked = sigismember(&t->blocked, sig);
+	if (blocked || ignored) {
+		action->sa.sa_handler = SIG_DFL;
+		if (blocked) {
+			sigdelset(&t->blocked, sig);
+			recalc_sigpending_tsk(t);
+		}
 	}
-	recalc_sigpending_tsk(t);
 	ret = specific_send_sig_info(sig, info, t);
 	spin_unlock_irqrestore(&t->sighand->siglock, flags);

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