lkml.org 
[lkml]   [2007]   [Oct]   [3]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
/
Date
From
SubjectRe: [PATCH 1/3] signal(i386): alternative signal stack wraparound occurs
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@cn.fujitsu.com>
>
> 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 majordomo@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/

\
 
 \ /
  Last update: 2007-10-03 14:23    [W:0.722 / U:0.004 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site