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 <[EMAIL PROTECTED]> > > 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 - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
