Thread (16 messages) 16 messages, 4 authors, 2021-05-18

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. Bae
quoted
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)



Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help