On Mon, Nov 22, 2021 at 4:31 AM <gregkh@xxxxxxxxxxxxxxxxxxx> wrote: > > > The patch below does not apply to the 5.15-stable tree. > If someone wants it applied there, or to any other stable or longterm > tree, then please email the backport, including the original git commit > id to <stable@xxxxxxxxxxxxxxx>. > > thanks, > > greg k-h > > ------------------ original commit in Linus's tree ------------------ > > From e349d945fac76bddc78ae1cb92a0145b427a87ce Mon Sep 17 00:00:00 2001 > From: "Eric W. Biederman" <ebiederm@xxxxxxxxxxxx> > Date: Thu, 18 Nov 2021 11:11:13 -0600 > Subject: [PATCH] signal: Don't always set SA_IMMUTABLE for forced signals > > Recently to prevent issues with SECCOMP_RET_KILL and similar signals > being changed before they are delivered SA_IMMUTABLE was added. > > Unfortunately this broke debuggers[1][2] which reasonably expect to be > able to trap synchronous SIGTRAP and SIGSEGV even when the target > process is not configured to handle those signals. > > Update force_sig_to_task to support both the case when we can allow > the debugger to intercept and possibly ignore the signal and the case > when it is not safe to let userspace know about the signal until the > process has exited. > > Suggested-by: Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> > Reported-by: Kyle Huey <me@xxxxxxxxxxxx> > Reported-by: kernel test robot <oliver.sang@xxxxxxxxx> > Cc: stable@xxxxxxxxxxxxxxx > [1] https://lkml.kernel.org/r/CAP045AoMY4xf8aC_4QU_-j7obuEPYgTcnQQP3Yxk=2X90jtpjw@xxxxxxxxxxxxxx > [2] https://lkml.kernel.org/r/20211117150258.GB5403@xsang-OptiPlex-9020 > Fixes: 00b06da29cf9 ("signal: Add SA_IMMUTABLE to ensure forced siganls do not get changed") > Link: https://lkml.kernel.org/r/877dd5qfw5.fsf_-_@xxxxxxxxxxxxxxxxxxxxxxxxxxxxxx > Reviewed-by: Kees Cook <keescook@xxxxxxxxxxxx> > Tested-by: Kees Cook <keescook@xxxxxxxxxxxx> > Tested-by: Kyle Huey <khuey@xxxxxxxxxxxx> > Signed-off-by: "Eric W. Biederman" <ebiederm@xxxxxxxxxxxx> > > diff --git a/kernel/signal.c b/kernel/signal.c > index 7c4b7ae714d4..7815e1bbeddc 100644 > --- a/kernel/signal.c > +++ b/kernel/signal.c > @@ -1298,6 +1298,12 @@ int do_send_sig_info(int sig, struct kernel_siginfo *info, struct task_struct *p > return ret; > } > > +enum sig_handler { > + HANDLER_CURRENT, /* If reachable use the current handler */ > + HANDLER_SIG_DFL, /* Always use SIG_DFL handler semantics */ > + HANDLER_EXIT, /* Only visible as the process exit code */ > +}; > + > /* > * Force a signal that the process can't ignore: if necessary > * we unblock the signal and change any SIG_IGN to SIG_DFL. > @@ -1310,7 +1316,8 @@ int do_send_sig_info(int sig, struct kernel_siginfo *info, struct task_struct *p > * that is why we also clear SIGNAL_UNKILLABLE. > */ > static int > -force_sig_info_to_task(struct kernel_siginfo *info, struct task_struct *t, bool sigdfl) > +force_sig_info_to_task(struct kernel_siginfo *info, struct task_struct *t, > + enum sig_handler handler) > { > unsigned long int flags; > int ret, blocked, ignored; > @@ -1321,9 +1328,10 @@ force_sig_info_to_task(struct kernel_siginfo *info, struct task_struct *t, bool > action = &t->sighand->action[sig-1]; > ignored = action->sa.sa_handler == SIG_IGN; > blocked = sigismember(&t->blocked, sig); > - if (blocked || ignored || sigdfl) { > + if (blocked || ignored || (handler != HANDLER_CURRENT)) { > action->sa.sa_handler = SIG_DFL; > - action->sa.sa_flags |= SA_IMMUTABLE; > + if (handler == HANDLER_EXIT) > + action->sa.sa_flags |= SA_IMMUTABLE; > if (blocked) { > sigdelset(&t->blocked, sig); > recalc_sigpending_and_wake(t); > @@ -1343,7 +1351,7 @@ force_sig_info_to_task(struct kernel_siginfo *info, struct task_struct *t, bool > > int force_sig_info(struct kernel_siginfo *info) > { > - return force_sig_info_to_task(info, current, false); > + return force_sig_info_to_task(info, current, HANDLER_CURRENT); > } > > /* > @@ -1660,7 +1668,7 @@ void force_fatal_sig(int sig) > info.si_code = SI_KERNEL; > info.si_pid = 0; > info.si_uid = 0; > - force_sig_info_to_task(&info, current, true); > + force_sig_info_to_task(&info, current, HANDLER_SIG_DFL); > } > > /* > @@ -1693,7 +1701,7 @@ int force_sig_fault_to_task(int sig, int code, void __user *addr > info.si_flags = flags; > info.si_isr = isr; > #endif > - return force_sig_info_to_task(&info, t, false); > + return force_sig_info_to_task(&info, t, HANDLER_CURRENT); > } > > int force_sig_fault(int sig, int code, void __user *addr > @@ -1813,7 +1821,8 @@ int force_sig_seccomp(int syscall, int reason, bool force_coredump) > info.si_errno = reason; > info.si_arch = syscall_get_arch(current); > info.si_syscall = syscall; > - return force_sig_info_to_task(&info, current, force_coredump); > + return force_sig_info_to_task(&info, current, > + force_coredump ? HANDLER_EXIT : HANDLER_CURRENT); > } > > /* For the crazy architectures that include trap information in > Since this is taken care of now, AFAICT, I do have one additional question. I reported the regression to LKML a day or so before 5.15.3 was cut. What should I have noticed to see that the regressing changeset was going to 5.15 and where should I have said "hey please don't ship this on 5.15 yet"? I'd like to know what to do next time :) - Kyle
