From 1612570d13675280a811b262ae69a988100f923a Mon Sep 17 00:00:00 2001 From: kib Date: Fri, 22 Aug 2014 07:52:47 +0000 Subject: Check the validity of struct sigaction sa_flags value, reject unknown flags. Sponsored by: The FreeBSD Foundation MFC after: 1 week --- sys/kern/kern_sig.c | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'sys/kern/kern_sig.c') diff --git a/sys/kern/kern_sig.c b/sys/kern/kern_sig.c index 561ea0a..31070f5 100644 --- a/sys/kern/kern_sig.c +++ b/sys/kern/kern_sig.c @@ -639,6 +639,10 @@ kern_sigaction(td, sig, act, oact, flags) if (!_SIG_VALID(sig)) return (EINVAL); + if (act != NULL && (act->sa_flags & ~(SA_ONSTACK | SA_RESTART | + SA_RESETHAND | SA_NOCLDSTOP | SA_NODEFER | SA_NOCLDWAIT | + SA_SIGINFO)) != 0) + return (EINVAL); PROC_LOCK(p); ps = p->p_sigacts; -- cgit v1.1 From 5b5e0178da19e2f8ed4687072a12f83056cbd219 Mon Sep 17 00:00:00 2001 From: kib Date: Fri, 22 Aug 2014 08:19:08 +0000 Subject: Ensure that sigaction flags for signal, which disposition is reset to ignored or default, are not leaking. Apparently, there exists code which relies on SA_SIGINFO not reported for SIG_DFL or SIG_IGN. In kern_sigaction, ignore flags when resetting. Encapsulate the flag and disposition testing into helper sigact_flag_test(). On exec, and when delivering signal with SA_RESETHAND flag set, signals are reset automatically. Use new helper sigdflt(), which removes duplicated code and corrects all flag bits for the signal. For proc0, set sigintr bit for all ignored signals. Ignored signals are consumed in tdsendsignal() and not delivered to the victim thread at all. Reported and tested by: royger Sponsored by: The FreeBSD Foundation MFC after: 1 week --- sys/kern/kern_sig.c | 77 ++++++++++++++++++++++++++++++----------------------- 1 file changed, 44 insertions(+), 33 deletions(-) (limited to 'sys/kern/kern_sig.c') diff --git a/sys/kern/kern_sig.c b/sys/kern/kern_sig.c index 31070f5..8810bf3 100644 --- a/sys/kern/kern_sig.c +++ b/sys/kern/kern_sig.c @@ -621,6 +621,15 @@ sig_ffs(sigset_t *set) return (0); } +static bool +sigact_flag_test(struct sigaction *act, int flag) +{ + + return ((act->sa_flags & flag) != 0 && + (__sighandler_t *)act->sa_sigaction != SIG_IGN && + (__sighandler_t *)act->sa_sigaction != SIG_DFL); +} + /* * kern_sigaction * sigaction @@ -683,7 +692,7 @@ kern_sigaction(td, sig, act, oact, flags) ps->ps_catchmask[_SIG_IDX(sig)] = act->sa_mask; SIG_CANTMASK(ps->ps_catchmask[_SIG_IDX(sig)]); - if (act->sa_flags & SA_SIGINFO) { + if (sigact_flag_test(act, SA_SIGINFO)) { ps->ps_sigact[_SIG_IDX(sig)] = (__sighandler_t *)act->sa_sigaction; SIGADDSET(ps->ps_siginfo, sig); @@ -691,19 +700,19 @@ kern_sigaction(td, sig, act, oact, flags) ps->ps_sigact[_SIG_IDX(sig)] = act->sa_handler; SIGDELSET(ps->ps_siginfo, sig); } - if (!(act->sa_flags & SA_RESTART)) + if (!sigact_flag_test(act, SA_RESTART)) SIGADDSET(ps->ps_sigintr, sig); else SIGDELSET(ps->ps_sigintr, sig); - if (act->sa_flags & SA_ONSTACK) + if (sigact_flag_test(act, SA_ONSTACK)) SIGADDSET(ps->ps_sigonstack, sig); else SIGDELSET(ps->ps_sigonstack, sig); - if (act->sa_flags & SA_RESETHAND) + if (sigact_flag_test(act, SA_RESETHAND)) SIGADDSET(ps->ps_sigreset, sig); else SIGDELSET(ps->ps_sigreset, sig); - if (act->sa_flags & SA_NODEFER) + if (sigact_flag_test(act, SA_NODEFER)) SIGADDSET(ps->ps_signodefer, sig); else SIGDELSET(ps->ps_signodefer, sig); @@ -904,14 +913,36 @@ siginit(p) PROC_LOCK(p); ps = p->p_sigacts; mtx_lock(&ps->ps_mtx); - for (i = 1; i <= NSIG; i++) - if (sigprop(i) & SA_IGNORE && i != SIGCONT) + for (i = 1; i <= NSIG; i++) { + if (sigprop(i) & SA_IGNORE && i != SIGCONT) { SIGADDSET(ps->ps_sigignore, i); + SIGADDSET(ps->ps_sigintr, i); + } + } mtx_unlock(&ps->ps_mtx); PROC_UNLOCK(p); } /* + * Reset specified signal to the default disposition. + */ +static void +sigdflt(struct sigacts *ps, int sig) +{ + + mtx_assert(&ps->ps_mtx, MA_OWNED); + SIGDELSET(ps->ps_sigcatch, sig); + if ((sigprop(sig) & SA_IGNORE) != 0 && sig != SIGCONT) + SIGADDSET(ps->ps_sigignore, sig); + ps->ps_sigact[_SIG_IDX(sig)] = SIG_DFL; + SIGDELSET(ps->ps_siginfo, sig); + SIGADDSET(ps->ps_sigintr, sig); + SIGDELSET(ps->ps_sigonstack, sig); + SIGDELSET(ps->ps_sigreset, sig); + SIGDELSET(ps->ps_signodefer, sig); +} + +/* * Reset signals for an exec of the specified process. */ void @@ -932,13 +963,9 @@ execsigs(struct proc *p) mtx_lock(&ps->ps_mtx); while (SIGNOTEMPTY(ps->ps_sigcatch)) { sig = sig_ffs(&ps->ps_sigcatch); - SIGDELSET(ps->ps_sigcatch, sig); - if (sigprop(sig) & SA_IGNORE) { - if (sig != SIGCONT) - SIGADDSET(ps->ps_sigignore, sig); + sigdflt(ps, sig); + if ((sigprop(sig) & SA_IGNORE) != 0) sigqueue_delete_proc(p, sig); - } - ps->ps_sigact[_SIG_IDX(sig)] = SIG_DFL; } /* * Reset stack state to the user stack. @@ -1892,16 +1919,8 @@ trapsignal(struct thread *td, ksiginfo_t *ksi) SIGADDSET(mask, sig); kern_sigprocmask(td, SIG_BLOCK, &mask, NULL, SIGPROCMASK_PROC_LOCKED | SIGPROCMASK_PS_LOCKED); - if (SIGISMEMBER(ps->ps_sigreset, sig)) { - /* - * See kern_sigaction() for origin of this code. - */ - SIGDELSET(ps->ps_sigcatch, sig); - if (sig != SIGCONT && - sigprop(sig) & SA_IGNORE) - SIGADDSET(ps->ps_sigignore, sig); - ps->ps_sigact[_SIG_IDX(sig)] = SIG_DFL; - } + if (SIGISMEMBER(ps->ps_sigreset, sig)) + sigdflt(ps, sig); mtx_unlock(&ps->ps_mtx); } else { /* @@ -2844,16 +2863,8 @@ postsig(sig) kern_sigprocmask(td, SIG_BLOCK, &mask, NULL, SIGPROCMASK_PROC_LOCKED | SIGPROCMASK_PS_LOCKED); - if (SIGISMEMBER(ps->ps_sigreset, sig)) { - /* - * See kern_sigaction() for origin of this code. - */ - SIGDELSET(ps->ps_sigcatch, sig); - if (sig != SIGCONT && - sigprop(sig) & SA_IGNORE) - SIGADDSET(ps->ps_sigignore, sig); - ps->ps_sigact[_SIG_IDX(sig)] = SIG_DFL; - } + if (SIGISMEMBER(ps->ps_sigreset, sig)) + sigdflt(ps, sig); td->td_ru.ru_nsignals++; if (p->p_sig == sig) { p->p_code = 0; -- cgit v1.1