Date: Tue, 15 Jun 2021 23:55:18 GMT From: Konstantin Belousov <kib@FreeBSD.org> To: src-committers@FreeBSD.org, dev-commits-src-all@FreeBSD.org, dev-commits-src-main@FreeBSD.org Subject: git: bc38762474ca - main - Add a knob to not drop signal with default ignored or ignored actions Message-ID: <202106152355.15FNtI0q086696@gitrepo.freebsd.org>
next in thread | raw e-mail | index | archive | help
The branch main has been updated by kib: URL: https://cgit.FreeBSD.org/src/commit/?id=bc38762474caed2d41d2562e28f56aa211f47ceb commit bc38762474caed2d41d2562e28f56aa211f47ceb Author: Konstantin Belousov <kib@FreeBSD.org> AuthorDate: 2021-06-05 12:42:27 +0000 Commit: Konstantin Belousov <kib@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))
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?202106152355.15FNtI0q086696>