git: bc38762474ca - main - Add a knob to not drop signal with default ignored or ignored actions

Konstantin Belousov kib at FreeBSD.org
Tue Jun 15 23:55:18 UTC 2021


The branch main has been updated by kib:

URL: https://cgit.FreeBSD.org/src/commit/?id=bc38762474caed2d41d2562e28f56aa211f47ceb

commit bc38762474caed2d41d2562e28f56aa211f47ceb
Author:     Konstantin Belousov <kib at FreeBSD.org>
AuthorDate: 2021-06-05 12:42:27 +0000
Commit:     Konstantin Belousov <kib at FreeBSD.org>
CommitDate: 2021-06-15 23:00:19 +0000

    Add a knob to not drop signal with default ignored or ignored actions
    
    Traditionally, BSD drops signals with the default action during send,
    not even putting them to the destination process queue.  This semantic
    is not shared with other operating systems (Linux), which do queue
    such signals.  In particular, sigtimedwait(2) and related syscalls can
    observe the delivery.
    
    Add a global knob kern.sig_discard_ign which can be set to false to force
    enqueuing of the signals with default action.  Also add an ABI flag to
    indicate that signals should be queued.
    
    Note that it is not practical to run with the knob turned on, because almost
    all software that care about the delivery of such signals, is aware of the
    difference, and misbehaves if the signals are actually queued.  The purpose
    of the knob as is is to allow for easier diagnostic of the programs that
    need the adjustments, to confirm the cause of problem.
    
    Reported by:    dchagin
    Reviewed by:    dchagin, markj
    Sponsored by:   The FreeBSD Foundation
    MFC after:      1 week
    Differential revision:  https://reviews.freebsd.org/D30675
---
 sys/kern/kern_sig.c | 65 +++++++++++++++++++++++++++++++----------------------
 sys/sys/proc.h      |  1 +
 sys/sys/sysent.h    |  1 +
 3 files changed, 40 insertions(+), 27 deletions(-)

diff --git a/sys/kern/kern_sig.c b/sys/kern/kern_sig.c
index 1cab25aa5a40..4f6f424fb05d 100644
--- a/sys/kern/kern_sig.c
+++ b/sys/kern/kern_sig.c
@@ -163,6 +163,12 @@ SYSCTL_BOOL(_kern, OID_AUTO, sigfastblock_fetch_always, CTLFLAG_RWTUN,
     "Fetch sigfastblock word on each syscall entry for proper "
     "blocking semantic");
 
+static bool	kern_sig_discard_ign = true;
+SYSCTL_BOOL(_kern, OID_AUTO, sig_discard_ign, CTLFLAG_RWTUN,
+    &kern_sig_discard_ign, 0,
+    "Discard ignored signals on delivery, otherwise queue them to "
+    "the target queue");
+
 SYSINIT(signal, SI_SUB_P1003_1B, SI_ORDER_FIRST+3, sigqueue_start, NULL);
 
 /*
@@ -1290,6 +1296,9 @@ kern_sigtimedwait(struct thread *td, sigset_t waitset, ksiginfo_t *ksi,
 	PROC_LOCK(p);
 	saved_mask = td->td_sigmask;
 	SIGSETNAND(td->td_sigmask, waitset);
+	if ((p->p_sysent->sv_flags & SV_SIG_DISCIGN) != 0 ||
+	    !kern_sig_discard_ign)
+		td->td_pflags2 |= TDP2_SIGWAIT;
 	for (;;) {
 		mtx_lock(&ps->ps_mtx);
 		sig = cursig(td);
@@ -1352,6 +1361,7 @@ kern_sigtimedwait(struct thread *td, sigset_t waitset, ksiginfo_t *ksi,
 		if (error == 0 && (p->p_ptevents & PTRACE_SYSCALL) != 0)
 			traced = true;
 	}
+	td->td_pflags2 &= ~TDP2_SIGWAIT;
 
 	new_block = saved_mask;
 	SIGSETNAND(new_block, td->td_sigmask);
@@ -2200,22 +2210,25 @@ tdsendsignal(struct proc *p, struct thread *td, int sig, ksiginfo_t *ksi)
 	SDT_PROBE3(proc, , , signal__send, td, p, sig);
 
 	/*
-	 * If the signal is being ignored,
-	 * then we forget about it immediately.
-	 * (Note: we don't set SIGCONT in ps_sigignore,
-	 * and if it is set to SIG_IGN,
-	 * action will be SIG_DFL here.)
+	 * If the signal is being ignored, then we forget about it
+	 * immediately, except when the target process executes
+	 * sigwait().  (Note: we don't set SIGCONT in ps_sigignore,
+	 * and if it is set to SIG_IGN, action will be SIG_DFL here.)
 	 */
 	mtx_lock(&ps->ps_mtx);
 	if (SIGISMEMBER(ps->ps_sigignore, sig)) {
-		SDT_PROBE3(proc, , , signal__discard, td, p, sig);
+		if (kern_sig_discard_ign &&
+		    (p->p_sysent->sv_flags & SV_SIG_DISCIGN) == 0) {
+			SDT_PROBE3(proc, , , signal__discard, td, p, sig);
 
-		mtx_unlock(&ps->ps_mtx);
-		if (ksi && (ksi->ksi_flags & KSI_INS))
-			ksiginfo_tryfree(ksi);
-		return (ret);
-	}
-	if (SIGISMEMBER(td->td_sigmask, sig))
+			mtx_unlock(&ps->ps_mtx);
+			if (ksi && (ksi->ksi_flags & KSI_INS))
+				ksiginfo_tryfree(ksi);
+			return (ret);
+		} else {
+			action = SIG_CATCH;
+		}
+	} else if (SIGISMEMBER(td->td_sigmask, sig))
 		action = SIG_HOLD;
 	else if (SIGISMEMBER(ps->ps_sigcatch, sig))
 		action = SIG_CATCH;
@@ -2950,11 +2963,13 @@ issignal(struct thread *td)
 		}
 
 		/*
-		 * We should see pending but ignored signals
-		 * only if P_TRACED was on when they were posted.
+		 * We should allow pending but ignored signals below
+		 * only if there is sigwait() active, or P_TRACED was
+		 * on when they were posted.
 		 */
 		if (SIGISMEMBER(ps->ps_sigignore, sig) &&
-		    (p->p_flag & P_TRACED) == 0) {
+		    (p->p_flag & P_TRACED) == 0 &&
+		    (td->td_pflags2 & TDP2_SIGWAIT) == 0) {
 			sigqueue_delete(&td->td_sigqueue, sig);
 			sigqueue_delete(&p->p_sigqueue, sig);
 			continue;
@@ -3066,10 +3081,11 @@ issignal(struct thread *td)
 				PROC_SUNLOCK(p);
 				mtx_lock(&ps->ps_mtx);
 				goto next;
-			} else if (prop & SIGPROP_IGNORE) {
+			} else if ((prop & SIGPROP_IGNORE) != 0 &&
+			    (td->td_pflags2 & TDP2_SIGWAIT) == 0) {
 				/*
-				 * Except for SIGCONT, shouldn't get here.
-				 * Default action is to ignore; drop it.
+				 * Default action is to ignore; drop it if
+				 * not in kern_sigtimedwait().
 				 */
 				break;		/* == ignore */
 			} else
@@ -3077,15 +3093,10 @@ issignal(struct thread *td)
 			/*NOTREACHED*/
 
 		case (intptr_t)SIG_IGN:
-			/*
-			 * Masking above should prevent us ever trying
-			 * to take action on an ignored signal other
-			 * than SIGCONT, unless process is traced.
-			 */
-			if ((prop & SIGPROP_CONT) == 0 &&
-			    (p->p_flag & P_TRACED) == 0)
-				printf("issignal\n");
-			break;		/* == ignore */
+			if ((td->td_pflags2 & TDP2_SIGWAIT) == 0)
+				break;		/* == ignore */
+			else
+				return (sig);
 
 		default:
 			/*
diff --git a/sys/sys/proc.h b/sys/sys/proc.h
index afa0be05d33a..ef27691ae4cd 100644
--- a/sys/sys/proc.h
+++ b/sys/sys/proc.h
@@ -529,6 +529,7 @@ do {									\
 #define	TDP2_SBPAGES	0x00000001 /* Owns sbusy on some pages */
 #define	TDP2_COMPAT32RB	0x00000002 /* compat32 ABI for robust lists */
 #define	TDP2_ACCT	0x00000004 /* Doing accounting */
+#define	TDP2_SIGWAIT	0x00000008 /* Ignore ignored signals */
 
 /*
  * Reasons that the current thread can not be run yet.
diff --git a/sys/sys/sysent.h b/sys/sys/sysent.h
index c2cbd77a92b9..95e9dcb1a335 100644
--- a/sys/sys/sysent.h
+++ b/sys/sys/sysent.h
@@ -161,6 +161,7 @@ struct sysentvec {
 #define	SV_TIMEKEEP	0x040000	/* Shared page timehands. */
 #define	SV_ASLR		0x080000	/* ASLR allowed. */
 #define	SV_RNG_SEED_VER	0x100000	/* random(4) reseed generation. */
+#define	SV_SIG_DISCIGN	0x200000	/* Do not discard ignored signals */
 
 #define	SV_ABI_MASK	0xff
 #define	SV_PROC_FLAG(p, x)	((p)->p_sysent->sv_flags & (x))


More information about the dev-commits-src-all mailing list