On Wed, 03 Oct 2007 17:06:24 +0900, Shi Weihua wrote:Fixing alternative signal stack wraparound.
If a process uses alternative signal stack by using sigaltstack()
and that stack overflow, stack wraparound occurs.
This patch checks whether the signal frame is on the alternative
stack. If the frame is not on there, kill a signal SIGSEGV to the process forcedly
then the process will be terminated.
This patch is for i386,version is 2.6.23-rc8.
Signed-off-by: Shi Weihua <shiwh@xxxxxxxxxxxxxx>
diff -pur linux-2.6.23-rc8.orig/arch/i386/kernel/signal.c linux-2.6.23-rc8/arch/i386/kernel/signal.c
--- linux-2.6.23-rc8.orig/arch/i386/kernel/signal.c 2007-09-26 09:44:08.000000000 +0900
+++ linux-2.6.23-rc8/arch/i386/kernel/signal.c 2007-09-26 13:14:25.000000000 +0900
@@ -332,6 +332,10 @@ static int setup_frame(int sig, struct k
frame = get_sigframe(ka, regs, sizeof(*frame));
+ if ((ka->sa.sa_flags & SA_ONSTACK) &&
+ !sas_ss_flags((unsigned long)frame))
+ goto give_sigsegv;
+
if (!access_ok(VERIFY_WRITE, frame, sizeof(*frame)))
goto give_sigsegv;
@@ -425,6 +429,10 @@ static int setup_rt_frame(int sig, struc
frame = get_sigframe(ka, regs, sizeof(*frame));
+ if ((ka->sa.sa_flags & SA_ONSTACK) &&
+ !sas_ss_flags((unsigned long)frame))
+ goto give_sigsegv;
+
if (!access_ok(VERIFY_WRITE, frame, sizeof(*frame)))
goto give_sigsegv;
Your patch description is a little terse. What you do is that
after the kernel has decided where to put the signal frame,
you add a check that the base of the frame still lies in the
altstack range if altstack delivery is requested for the signal,
and if it doesn't a hard error is forced.
The coding of that logic is fine.
What I don't agree with is the logic itself:
- You only catch altstack overflow caused by the kernel pushing
a sigframe. You don't catch overflow caused by the user-space
signal handler pushing its own stack frame after the sigframe.
- SUSv3 specifies the effect of altstack overflow as "undefined".
- The overflow problem can be solved in user-space: allocate the
altstack with mmap(), then mprotect() the lowest page to prevent
accesses to it. Any overflow into it, by the kernel's signal
delivery code or by the user-space signal handler, will be caught.
So this patch gets a NAK from me.
/Mikael