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/
[Index of Archives]
[Kernel Newbies]
[Netfilter]
[Bugtraq]
[Photo]
[Stuff]
[Gimp]
[Yosemite News]
[MIPS Linux]
[ARM Linux]
[Linux Security]
[Linux RAID]
[Video 4 Linux]
[Linux for the blind]
[Linux Resources]
