avoid spinning processes when catching SEGV/BUS/FPE

[christos%zoulas.com@localhost (Christos Zoulas)]

Hello,

Currently processes that catch SIGSEGV/SIGBUS/SIGFPE and receive
a signal again while they are processing the signal handler end in
an infinite loop spinning because the signal is masked during delivery
(and executing the handler).

This is not desirable behavior. I would be better to just reset the
signal to default if it is being "caught and masked" since in that
scenario the process will never see the signal and the signal will
keep being delivered.

This has also the unfortunate side effect that makes the problem
undebuggable (aside from wasting CPU resources). The following patch
fixes that.

Here's a contrived example:

$ cat << EOF > spinner.c
#include <signal.h>
#include <stdlib.h>
#include <stdio.h>
#include <unistd.h>

static void
foo(int s)
{
        char buf[64];
        int i = snprintf(buf, sizeof(buf), "got %d\n", s);
        write(2, buf, i);

	// This will segv because we'll try to deref 11
        i = snprintf(buf, sizeof(buf), "got %s\n", s);
        write(2, buf, i);
        exit(0);
}

int
main(void)
{
        int *p = (int *) 0xfefefef0;
        struct sigaction sa;
        sa.sa_flags = SA_RESTART;
        sa.sa_handler = foo;
        sigemptyset(&sa.sa_mask);
        sigaction(SIGSEGV, &sa, NULL);
        *p = 1;
}
EOF

Opinions?

christos


Index: sys/signalvar.h
===================================================================
RCS file: /cvsroot/src/sys/sys/signalvar.h,v
retrieving revision 1.88
diff -u -p -u -r1.88 signalvar.h
--- sys/signalvar.h	6 Jan 2017 22:53:17 -0000	1.88
+++ sys/signalvar.h	6 Dec 2017 23:14:14 -0000
@@ -107,6 +107,7 @@ struct sigctx {
 #define	SA_NORESET	0x0080		/* not reset when caught */
 #define	SA_TOLWP	0x0100		/* to LWP that generated, if local */
 #define	SA_TOALL	0x0200		/* always to all LWPs */
+#define	SA_RESET	0x0400		/* reset when caught and masked */
 
 #ifdef _KERNEL
 
@@ -247,10 +248,10 @@ const int sigprop[NSIG] = {
 	SA_KILL|SA_CORE|SA_NORESET|SA_TOLWP,	/* 5 SIGTRAP */
 	SA_KILL|SA_CORE,			/* 6 SIGABRT */
 	SA_KILL|SA_CORE|SA_TOLWP,		/* 7 SIGEMT */
-	SA_KILL|SA_CORE|SA_TOLWP,		/* 8 SIGFPE */
+	SA_KILL|SA_CORE|SA_TOLWP|SA_RESET,	/* 8 SIGFPE */
 	SA_KILL|SA_CANTMASK|SA_TOALL,		/* 9 SIGKILL */
-	SA_KILL|SA_CORE|SA_TOLWP,		/* 10 SIGBUS */
-	SA_KILL|SA_CORE|SA_TOLWP,		/* 11 SIGSEGV */
+	SA_KILL|SA_CORE|SA_TOLWP|SA_RESET,	/* 10 SIGBUS */
+	SA_KILL|SA_CORE|SA_TOLWP|SA_RESET,	/* 11 SIGSEGV */
 	SA_KILL|SA_CORE|SA_TOLWP,		/* 12 SIGSYS */
 	SA_KILL,				/* 13 SIGPIPE */
 	SA_KILL,				/* 14 SIGALRM */
Index: kern/kern_sig.c
===================================================================
RCS file: /cvsroot/src/sys/kern/kern_sig.c,v
retrieving revision 1.338
diff -u -p -u -r1.338 kern_sig.c
--- kern/kern_sig.c	25 Oct 2017 08:12:39 -0000	1.338
+++ kern/kern_sig.c	6 Dec 2017 23:14:15 -0000
@@ -903,9 +903,10 @@ trapsignal(struct lwp *l, ksiginfo_t *ks
 	mask = &l->l_sigmask;
 	ps = p->p_sigacts;
 
-	if ((p->p_slflag & PSL_TRACED) == 0 &&
-	    sigismember(&p->p_sigctx.ps_sigcatch, signo) &&
-	    !sigismember(mask, signo)) {
+	const bool traced = (p->p_slflag & PSL_TRACED) != 0;
+	const bool caught = sigismember(&p->p_sigctx.ps_sigcatch, signo);
+	const bool masked = sigismember(mask, signo);
+	if (!traced && caught && !masked) {
 		mutex_exit(proc_lock);
 		l->l_ru.ru_nsignals++;
 		kpsendsig(l, ksi, mask);
@@ -920,11 +921,19 @@ trapsignal(struct lwp *l, ksiginfo_t *ks
 				    SIGACTION_PS(ps, signo).sa_handler,
 				    mask, ksi);
 		}
-	} else {
-		kpsignal2(p, ksi);
-		mutex_exit(p->p_lock);
-		mutex_exit(proc_lock);
+		return;
+	}
+
+	if (!traced && caught && masked && (sigprop[signo] & SA_RESET)) {
+		sigdelset(mask, signo);	
+		sigdelset(&SIGACTION_PS(ps, signo).sa_mask, signo);
+		sigdelset(&p->p_sigctx.ps_sigcatch, signo);
+		sigdelset(&p->p_sigctx.ps_sigignore, signo);
+		SIGACTION_PS(ps, signo).sa_handler = SIG_DFL;
 	}
+	kpsignal2(p, ksi);
+	mutex_exit(p->p_lock);
+	mutex_exit(proc_lock);
 }
 
 /*