Re: [PATCH v8 5/6] x86/signal: Detect and prevent an alternate signal stack overflow
From: Bae, Chang Seok <hidden>
Date: 2021-04-22 16:31:59
Also in:
linux-arch, lkml
On Apr 22, 2021, at 01:46, David Laight [off-list ref] wrote:
From: Chang S. Baequoted
Sent: 22 April 2021 05:49 The kernel pushes context on to the userspace stack to prepare for the user's signal handler. When the user has supplied an alternate signal stack, via sigaltstack(2), it is easy for the kernel to verify that the stack size is sufficient for the current hardware context. Check if writing the hardware context to the alternate stack will exceed it's size. If yes, then instead of corrupting user-data and proceeding with the original signal handler, an immediate SIGSEGV signal is delivered.What happens if SIGSEGV is caught?
Boris pointed out the relevant notes before [1]. I think "unpredictable results" is a somewhat vague statement but process termination is unavoidable in this situation. In the thread [1], a new signal number was discussed for the signal delivery failure, but my takeaway is this SIGSEGV is still recognizable. FWIW, Len summarized other possible approaches as well [2].
quoted
Refactor the stack pointer check code from on_sig_stack() and use the new helper. While the kernel allows new source code to discover and use a sufficient alternate signal stack size, this check is still necessary to protect binaries with insufficient alternate signal stack size from data corruption....quoted
diff --git a/include/linux/sched/signal.h b/include/linux/sched/signal.h index 3f6a0fcaa10c..ae60f838ebb9 100644 --- a/include/linux/sched/signal.h +++ b/include/linux/sched/signal.h@@ -537,6 +537,17 @@ static inline int kill_cad_pid(int sig, int priv)#define SEND_SIG_NOINFO ((struct kernel_siginfo *) 0) #define SEND_SIG_PRIV ((struct kernel_siginfo *) 1) +static inline int __on_sig_stack(unsigned long sp) +{ +#ifdef CONFIG_STACK_GROWSUP + return sp >= current->sas_ss_sp && + sp - current->sas_ss_sp < current->sas_ss_size; +#else + return sp > current->sas_ss_sp && + sp - current->sas_ss_sp <= current->sas_ss_size; +#endif +} +Those don't look different enough.
The difference is on the SS_AUTODISARM flag check. This refactoring was suggested as on_sig_stack() brought confusion [3]. Thanks, Chang [1] https://lore.kernel.org/lkml/20210414120608.GE10709@zn.tnic/ (local) [2] https://lore.kernel.org/lkml/CAJvTdKnpWL8y4N_BrCiK7fU0UXERwuuM8o84LUpp7Watxd8STw@mail.gmail.com/ (local) [3] https://lore.kernel.org/lkml/20210325212733.GC32296@zn.tnic/ (local)