Re: [PATCH 1/5] signal(x86_32): Improve the signal stack overflow check
Roland McGrath wrote:: >> I spent some time read you mail carefully and dig into the code again. >> >> And yes, you are right. It's possible that SA_ONSTACK has been cleared >> before the second signal on the same stack comes. > > It's not necessary for SA_ONSTACK to have "been cleared", by which I assume > you mean a sigaction call with SA_ONSTACK not set in sa_flags. That is > indeed possible, but it's not the only case your patch broke. It can just > be a different signal whose sigaction never had SA_ONSTACK, when you are > still on the signal stack from an earlier signal that did have SA_ONSTACK. Thanks for your explanation. > >> So this patch is wrong :( . I will revise the other 4 patches. > > For 2 and 3, I would rather just wait until we unify signal.c anyway. Ok. I see. -- 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/
Re: [PATCH 1/5] signal(x86_32): Improve the signal stack overflow check
Roland McGrath wrote:: >> You mean the comment? > > No, that is trivial and already corrected. I mean the substance of your > most recent patch. I described why I think it is wrong. You did not respond. I spent some time read you mail carefully and dig into the code again. And yes, you are right. It's possible that SA_ONSTACK has been cleared before the second signal on the same stack comes. So this patch is wrong :( . I will revise the other 4 patches. Sorry for the noise. -- 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/
Re: [PATCH 1/5] signal(x86_32): Improve the signal stack overflow check
Roland McGrath wrote:: >>> Please elaborate on the rationale that justifies this change. >>> I don't see it at all. >> I have corrected the comment in the latest patch which has been apllied by >> Ingo. >> Please refer to http://lkml.org/lkml/2008/2/18/575 and >> http://lkml.org/lkml/2008/2/19/119 . > > You have yet to say anything to justify your change. You mean the comment? I mean I made a mistake about the comment so I changed it, and then Valdis.Kletnieks pointed it out for me. And late I sent a revised patch with your original comment untouched. Sorry for my poor English. :( Thanks Shi Weihua -- 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/
Re: [PATCH 1/5] signal(x86_32): Improve the signal stack overflow check
Roland McGrath wrote:: > This change looks bogus to me. Before I get to the content, there is a nit > that annoys me. You changed the punctuation in my comment so that it no > longer means what it did, and now the comment is nonsensical. I don't > demand decent English from hackers of any linguistic bent, but please don't > louse up the coherent sentences I wrote when moving them down ten lines. > > Please elaborate on the rationale that justifies this change. > I don't see it at all. I have corrected the comment in the latest patch which has been apllied by Ingo. Please refer to http://lkml.org/lkml/2008/2/18/575 and http://lkml.org/lkml/2008/2/19/119 . Thanks. Shi Weihua > > If you are already on the signal stack, it doesn't matter whether the > signal that just arrived has SA_ONSTACK set or not. If you are going to > overflow the stack with the new signal frame, we want to prevent that > clobberation regardless. > > > Thanks, > Roland > > > -- 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/
Re: [PATCH 5/5] signal(ia64_ia32): add a signal stack overflow check
The similar check has been added to x86_32(i386) in commit
id 83bd01024b1fdfc41d9b758e5669e80fca72df66.
So we add this check to ia64_ia32 and improve it a liitle bit in that
we need to check for stack overflow only when the signal is on stack.
Signed-off-by: Shi Weihua <[EMAIL PROTECTED]>
---
The previous patch has a comment mistake. Now I correct it.
---
--- linux-2.6.25-rc2.orig/arch/ia64/ia32/ia32_signal.c 2008-02-16
04:57:20.0 +0800
+++ linux-2.6.25-rc2/arch/ia64/ia32/ia32_signal.c 2008-02-19
09:57:28.0 +0800
@@ -766,8 +766,19 @@ get_sigframe (struct k_sigaction *ka, st
/* This is the X/Open sanctioned signal stack switching. */
if (ka->sa.sa_flags & SA_ONSTACK) {
- if (!on_sig_stack(esp))
+ int onstack = sas_ss_flags(esp);
+
+ if (onstack == 0)
esp = current->sas_ss_sp + current->sas_ss_size;
+ else if (onstack == SS_ONSTACK) {
+ /*
+* If we are on the alternate signal stack and would
+* overflow it, don't. Return an always-bogus address
+* instead so we will die with SIGSEGV.
+*/
+ if (!likely(on_sig_stack(esp - frame_size)))
+ return (void __user *) -1L;
+ }
}
/* Legacy stack switching not supported */
--
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/
Re: [PATCH 4/5] signal(ia64): add a signal stack overflow check
The similar check has been added to x86_32(i386) in commit
id 83bd01024b1fdfc41d9b758e5669e80fca72df66.
So we add this check to ia64 and improve it a liitle bit in that
we need to check for stack overflow only when the signal is on stack.
Signed-off-by: Shi Weihua <[EMAIL PROTECTED]>
---
The previous patch has a comment mistake. Now I correct it.
---
--- linux-2.6.25-rc2.orig/arch/ia64/kernel/signal.c 2008-02-16
04:57:20.0 +0800
+++ linux-2.6.25-rc2/arch/ia64/kernel/signal.c 2008-02-19 09:57:05.0
+0800
@@ -342,15 +342,33 @@ setup_frame (int sig, struct k_sigaction
new_sp = scr->pt.r12;
tramp_addr = (unsigned long) __kernel_sigtramp;
- if ((ka->sa.sa_flags & SA_ONSTACK) && sas_ss_flags(new_sp) == 0) {
- new_sp = current->sas_ss_sp + current->sas_ss_size;
- /*
-* We need to check for the register stack being on the signal
stack
-* separately, because it's switched separately (memory stack
is switched
-* in the kernel, register stack is switched in the signal
trampoline).
-*/
- if (!rbs_on_sig_stack(scr->pt.ar_bspstore))
- new_rbs = (current->sas_ss_sp + sizeof(long) - 1) &
~(sizeof(long) - 1);
+ if (ka->sa.sa_flags & SA_ONSTACK) {
+ int onstack = sas_ss_flags(new_sp);
+
+ if (onstack == 0) {
+ new_sp = current->sas_ss_sp + current->sas_ss_size;
+ /*
+* We need to check for the register stack being on the
+* signal stack separately, because it's switched
+* separately (memory stack is switched in the kernel,
+* register stack is switched in the signal trampoline).
+*/
+ if (!rbs_on_sig_stack(scr->pt.ar_bspstore))
+ new_rbs = ALIGN(current->sas_ss_sp,
+ sizeof(long));
+ } else if (onstack == SS_ONSTACK) {
+ unsigned long check_sp;
+
+ /*
+* If we are on the alternate signal stack and would
+* overflow it, don't. Return an always-bogus address
+* instead so we will die with SIGSEGV.
+*/
+ check_sp = (new_sp - sizeof(*frame)) & -STACK_ALIGN;
+ if (!likely(on_sig_stack(check_sp)))
+ return force_sigsegv_info(sig, (void __user *)
+ check_sp);
+ }
}
frame = (void __user *) ((new_sp - sizeof(*frame)) & -STACK_ALIGN);
--
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/
Re: [PATCH 3/5] signal(x86_ia32): add a signal stack overflow check
The similar check has been added to x86_32(i386) in commit
id 83bd01024b1fdfc41d9b758e5669e80fca72df66.
So we add this check to x86_ia32 and improve it a liitle bit in that
we need to check for stack overflow only when the signal is on stack.
Signed-off-by: Shi Weihua <[EMAIL PROTECTED]>
---
The previous patch has a comment mistake. Now I correct it.
---
--- linux-2.6.25-rc2.orig/arch/x86/ia32/ia32_signal.c 2008-02-16
04:57:20.0 +0800
+++ linux-2.6.25-rc2/arch/x86/ia32/ia32_signal.c2008-02-19
09:56:43.0 +0800
@@ -406,8 +406,19 @@ static void __user *get_sigframe(struct
/* This is the X/Open sanctioned signal stack switching. */
if (ka->sa.sa_flags & SA_ONSTACK) {
- if (sas_ss_flags(sp) == 0)
+ int onstack = sas_ss_flags(sp);
+
+ if (onstack == 0)
sp = current->sas_ss_sp + current->sas_ss_size;
+ else if (onstack == SS_ONSTACK) {
+ /*
+* If we are on the alternate signal stack and would
+* overflow it, don't. Return an always-bogus address
+* instead so we will die with SIGSEGV.
+*/
+if (!likely(on_sig_stack(sp - frame_size)))
+ return (void __user *) -1L;
+ }
}
/* This is the legacy signal stack switching. */
--
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/
Re: [PATCH 2/5] signal(x86_64): add a signal stack overflow check
The similar check has been added to x86_32(i386) in commit
id 83bd01024b1fdfc41d9b758e5669e80fca72df66.
So we add this check to x86_64 and improve it a liitle bit in that
we need to check for stack overflow only when the signal is on stack.
Signed-off-by: Shi Weihua <[EMAIL PROTECTED]>
---
The previous patch has a comment mistake. Now I correct it.
---
--- linux-2.6.25-rc2.orig/arch/x86/kernel/signal_64.c 2008-02-16
04:57:20.0 +0800
+++ linux-2.6.25-rc2/arch/x86/kernel/signal_64.c2008-02-19
09:56:20.0 +0800
@@ -205,8 +205,19 @@ get_stack(struct k_sigaction *ka, struct
/* This is the X/Open sanctioned signal stack switching. */
if (ka->sa.sa_flags & SA_ONSTACK) {
- if (sas_ss_flags(sp) == 0)
+ int onstack = sas_ss_flags(sp);
+
+ if (onstack == 0)
sp = current->sas_ss_sp + current->sas_ss_size;
+ else if (onstack == SS_ONSTACK) {
+ /*
+* If we are on the alternate signal stack and would
+* overflow it, don't. Return an always-bogus address
+* instead so we will die with SIGSEGV.
+*/
+ if (!likely(on_sig_stack(sp - size)))
+ return (void __user *) -1L;
+ }
}
return (void __user *)round_down(sp - size, 16);
--
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/
Re: [PATCH 1/5] signal(x86_32): Improve the signal stack overflow check
We need to check for stack overflow only when the signal is on stack.
So we can improve the patch "http://lkml.org/lkml/2007/11/27/101"; as following.
Signed-off-by: Shi Weihua <[EMAIL PROTECTED]>
---
The previous patch has a comment mistake. Now I correct it.
---
--- linux-2.6.25-rc2.orig/arch/x86/kernel/signal_32.c 2008-02-16
04:57:20.0 +0800
+++ linux-2.6.25-rc2/arch/x86/kernel/signal_32.c2008-02-19
09:55:59.0 +0800
@@ -299,17 +299,21 @@ get_sigframe(struct k_sigaction *ka, str
/* Default to using normal stack */
sp = regs->sp;
- /*
-* If we are on the alternate signal stack and would overflow it, don't.
-* Return an always-bogus address instead so we will die with SIGSEGV.
-*/
- if (on_sig_stack(sp) && !likely(on_sig_stack(sp - frame_size)))
- return (void __user *) -1L;
-
/* This is the X/Open sanctioned signal stack switching. */
if (ka->sa.sa_flags & SA_ONSTACK) {
- if (sas_ss_flags(sp) == 0)
+ int onstack = sas_ss_flags(sp);
+
+ if (onstack == 0)
sp = current->sas_ss_sp + current->sas_ss_size;
+ else if (onstack == SS_ONSTACK) {
+ /*
+* If we are on the alternate signal stack and would
+* overflow it, don't. Return an always-bogus address
+* instead so we will die with SIGSEGV.
+*/
+ if (!likely(on_sig_stack(sp - frame_size)))
+ return (void __user *) -1L;
+ }
}
/* This is the legacy signal stack switching. */
--
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/
Re: [PATCH 1/5] signal(x86_32): Improve the signal stack overflow check
[EMAIL PROTECTED] wrote:: > On Mon, 18 Feb 2008 18:22:05 +0800, Shi Weihua said: > >> -/* >> - * If we are on the alternate signal stack and would overflow it, don't. >notice > this ^ >> - * Return an always-bogus address instead so we will die with SIGSEGV. > >> + * If we are on the alternate signal stack and would >> + * overflow it, don't return an always-bogus address > missing here ^ >> + * instead so we will die with SIGSEGV. > > "don't. Return" is a vastly different semantic than "don't return". > > I think the same comment error was cut-n-pasted into all 5 patches... > Sorry, it's my mistake. I will correct the all 5 patches. Thanks. -- 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/
Re: [PATCH 4/5] signal(ia64): add a signal stack overflow check
Matthew Wilcox wrote::
> On Mon, Feb 18, 2008 at 06:26:23PM +0800, Shi Weihua wrote:
>> +if (!rbs_on_sig_stack(scr->pt.ar_bspstore))
>> +new_rbs = (current->sas_ss_sp +
>> + sizeof(long) - 1) & ~(sizeof(long) -
>> 1);
>
> I know you're only moving this code, but how about fixing it to use
> ALIGN at the same time?
>
> + if (!rbs_on_sig_stack(scr->pt.ar_bspstore))
> + new_rbs = ALIGN(current->sas_ss_sp,
> + sizeof(long));
>
Of course,we can improve the code by ALIGN.
---
The similar check has been added to x86_32(i386) in commit
id 83bd01024b1fdfc41d9b758e5669e80fca72df66.
So we add this check to ia64 and improve it a liitle bit in that
we need to check for stack overflow only when the signal is on stack.
Signed-off-by: Shi Weihua <[EMAIL PROTECTED]>
---
--- linux-2.6.25-rc2.orig/arch/ia64/kernel/signal.c 2008-02-16
04:57:20.0 +0800
+++ linux-2.6.25-rc2/arch/ia64/kernel/signal.c 2008-02-19 09:57:05.0
+0800
@@ -342,15 +342,33 @@ setup_frame (int sig, struct k_sigaction
new_sp = scr->pt.r12;
tramp_addr = (unsigned long) __kernel_sigtramp;
- if ((ka->sa.sa_flags & SA_ONSTACK) && sas_ss_flags(new_sp) == 0) {
- new_sp = current->sas_ss_sp + current->sas_ss_size;
- /*
-* We need to check for the register stack being on the signal
stack
-* separately, because it's switched separately (memory stack
is switched
-* in the kernel, register stack is switched in the signal
trampoline).
-*/
- if (!rbs_on_sig_stack(scr->pt.ar_bspstore))
- new_rbs = (current->sas_ss_sp + sizeof(long) - 1) &
~(sizeof(long) - 1);
+ if (ka->sa.sa_flags & SA_ONSTACK) {
+ int onstack = sas_ss_flags(new_sp);
+
+ if (onstack == 0) {
+ new_sp = current->sas_ss_sp + current->sas_ss_size;
+ /*
+* We need to check for the register stack being on the
+* signal stack separately, because it's switched
+* separately (memory stack is switched in the kernel,
+* register stack is switched in the signal trampoline).
+*/
+ if (!rbs_on_sig_stack(scr->pt.ar_bspstore))
+ new_rbs = ALIGN(current->sas_ss_sp,
+ sizeof(long));
+ } else if (onstack == SS_ONSTACK) {
+ unsigned long check_sp;
+
+ /*
+* If we are on the alternate signal stack and would
+* overflow it, don't. Return an always-bogus address
+* instead so we will die with SIGSEGV.
+*/
+ check_sp = (new_sp - sizeof(*frame)) & -STACK_ALIGN;
+ if (!likely(on_sig_stack(check_sp)))
+ return force_sigsegv_info(sig, (void __user *)
+ check_sp);
+ }
}
frame = (void __user *) ((new_sp - sizeof(*frame)) & -STACK_ALIGN);
--
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/
Re: [PATCH 1/5] signal(x86_32): Improve the signal stack overflow check
Ingo Molnar wrote:: > * Shi Weihua <[EMAIL PROTECTED]> wrote: > >> We need to check for stack overflow only when the signal is on stack. >> So we can improve the patch "http://lkml.org/lkml/2007/11/27/101"; as >> following. > > hm, does this address Roland's observations at: > >http://lkml.org/lkml/2007/11/28/13 > > ? Yes. please check it. > > Ingo > > > -- 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/
[PATCH 5/5] signal(ia64_ia32): add a signal stack overflow check
The similar check has been added to x86_32(i386) in commit
id 83bd01024b1fdfc41d9b758e5669e80fca72df66.
So we add this check to ia64_ia32 and improve it a liitle bit in that
we need to check for stack overflow only when the signal is on stack.
Signed-off-by: Shi Weihua <[EMAIL PROTECTED]>
---
--- linux-2.6.25-rc2.orig/arch/ia64/ia32/ia32_signal.c 2008-02-16
04:57:20.0 +0800
+++ linux-2.6.25-rc2/arch/ia64/ia32/ia32_signal.c 2008-02-18
18:12:49.0 +0800
@@ -766,8 +766,19 @@ get_sigframe (struct k_sigaction *ka, st
/* This is the X/Open sanctioned signal stack switching. */
if (ka->sa.sa_flags & SA_ONSTACK) {
- if (!on_sig_stack(esp))
+ int onstack = sas_ss_flags(esp);
+
+ if (onstack == 0)
esp = current->sas_ss_sp + current->sas_ss_size;
+ else if (onstack == SS_ONSTACK) {
+ /*
+* If we are on the alternate signal stack and would
+* overflow it, don't return an always-bogus address
+* instead so we will die with SIGSEGV.
+*/
+ if (!likely(on_sig_stack(esp - frame_size)))
+ return (void __user *) -1L;
+ }
}
/* Legacy stack switching not supported */
--
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/
[PATCH 4/5] signal(ia64): add a signal stack overflow check
The similar check has been added to x86_32(i386) in commit
id 83bd01024b1fdfc41d9b758e5669e80fca72df66.
So we add this check to ia64 and improve it a liitle bit in that
we need to check for stack overflow only when the signal is on stack.
Signed-off-by: Shi Weihua <[EMAIL PROTECTED]>
---
--- linux-2.6.25-rc2.orig/arch/ia64/kernel/signal.c 2008-02-16
04:57:20.0 +0800
+++ linux-2.6.25-rc2/arch/ia64/kernel/signal.c 2008-02-18 18:17:15.0
+0800
@@ -342,15 +342,29 @@ setup_frame (int sig, struct k_sigaction
new_sp = scr->pt.r12;
tramp_addr = (unsigned long) __kernel_sigtramp;
- if ((ka->sa.sa_flags & SA_ONSTACK) && sas_ss_flags(new_sp) == 0) {
- new_sp = current->sas_ss_sp + current->sas_ss_size;
- /*
-* We need to check for the register stack being on the signal
stack
-* separately, because it's switched separately (memory stack
is switched
-* in the kernel, register stack is switched in the signal
trampoline).
-*/
- if (!rbs_on_sig_stack(scr->pt.ar_bspstore))
- new_rbs = (current->sas_ss_sp + sizeof(long) - 1) &
~(sizeof(long) - 1);
+ if (ka->sa.sa_flags & SA_ONSTACK) {
+ int onstack = sas_ss_flags(new_sp);
+
+ if (onstack == 0) {
+ new_sp = current->sas_ss_sp + current->sas_ss_size;
+ /*
+* We need to check for the register stack being on the
+* signal stack separately, because it's switched
+* separately (memory stack is switched in the kernel,
+* register stack is switched in the signal trampoline).
+*/
+ if (!rbs_on_sig_stack(scr->pt.ar_bspstore))
+ new_rbs = (current->sas_ss_sp +
+ sizeof(long) - 1) & ~(sizeof(long) -
1);
+ } else if (onstack == SS_ONSTACK) {
+ /*
+* If we are on the alternate signal stack and would
+* overflow it, don't return an always-bogus address
+* instead so we will die with SIGSEGV.
+*/
+ if (!likely(on_sig_stack(new_sp - sizeof(*frame
+ return force_sigsegv_info(sig, frame);
+ }
}
frame = (void __user *) ((new_sp - sizeof(*frame)) & -STACK_ALIGN);
--
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/
[PATCH 2/5] signal(x86_64): add a signal stack overflow check
The similar check has been added to x86_32(i386) in commit
id 83bd01024b1fdfc41d9b758e5669e80fca72df66.
So we add this check to x86_64 and improve it a liitle bit in that
we need to check for stack overflow only when the signal is on stack.
Signed-off-by: Shi Weihua <[EMAIL PROTECTED]>
---
--- linux-2.6.25-rc2.orig/arch/x86/kernel/signal_64.c 2008-02-16
04:57:20.0 +0800
+++ linux-2.6.25-rc2/arch/x86/kernel/signal_64.c2008-02-18
18:06:48.0 +0800
@@ -205,8 +205,19 @@ get_stack(struct k_sigaction *ka, struct
/* This is the X/Open sanctioned signal stack switching. */
if (ka->sa.sa_flags & SA_ONSTACK) {
- if (sas_ss_flags(sp) == 0)
+ int onstack = sas_ss_flags(sp);
+
+ if (onstack == 0)
sp = current->sas_ss_sp + current->sas_ss_size;
+ else if (onstack == SS_ONSTACK) {
+ /*
+* If we are on the alternate signal stack and would
+* overflow it, don't return an always-bogus address
+* instead so we will die with SIGSEGV.
+*/
+ if (!likely(on_sig_stack(sp - size)))
+ return (void __user *) -1L;
+ }
}
return (void __user *)round_down(sp - size, 16);
--
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/
[PATCH 3/5] signal(x86_ia32): add a signal stack overflow check
The similar check has been added to x86_32(i386) in commit
id 83bd01024b1fdfc41d9b758e5669e80fca72df66.
So we add this check to x86_ia32 and improve it a liitle bit in that
we need to check for stack overflow only when the signal is on stack.
Signed-off-by: Shi Weihua <[EMAIL PROTECTED]>
---
--- linux-2.6.25-rc2.orig/arch/x86/ia32/ia32_signal.c 2008-02-16
04:57:20.0 +0800
+++ linux-2.6.25-rc2/arch/x86/ia32/ia32_signal.c2008-02-18
18:07:01.0 +0800
@@ -406,8 +406,19 @@ static void __user *get_sigframe(struct
/* This is the X/Open sanctioned signal stack switching. */
if (ka->sa.sa_flags & SA_ONSTACK) {
- if (sas_ss_flags(sp) == 0)
+ int onstack = sas_ss_flags(sp);
+
+ if (onstack == 0)
sp = current->sas_ss_sp + current->sas_ss_size;
+ else if (onstack == SS_ONSTACK) {
+ /*
+* If we are on the alternate signal stack and would
+* overflow it, don't return an always-bogus address
+* instead so we will die with SIGSEGV.
+*/
+if (!likely(on_sig_stack(sp - frame_size)))
+ return (void __user *) -1L;
+ }
}
/* This is the legacy signal stack switching. */
--
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/
[PATCH 1/5] signal(x86_32): Improve the signal stack overflow check
We need to check for stack overflow only when the signal is on stack.
So we can improve the patch "http://lkml.org/lkml/2007/11/27/101"; as following.
Signed-off-by: Shi Weihua <[EMAIL PROTECTED]>
---
--- linux-2.6.25-rc2.orig/arch/x86/kernel/signal_32.c 2008-02-16
04:57:20.0 +0800
+++ linux-2.6.25-rc2/arch/x86/kernel/signal_32.c2008-02-18
18:06:39.0 +0800
@@ -299,17 +299,21 @@ get_sigframe(struct k_sigaction *ka, str
/* Default to using normal stack */
sp = regs->sp;
- /*
-* If we are on the alternate signal stack and would overflow it, don't.
-* Return an always-bogus address instead so we will die with SIGSEGV.
-*/
- if (on_sig_stack(sp) && !likely(on_sig_stack(sp - frame_size)))
- return (void __user *) -1L;
-
/* This is the X/Open sanctioned signal stack switching. */
if (ka->sa.sa_flags & SA_ONSTACK) {
- if (sas_ss_flags(sp) == 0)
+ int onstack = sas_ss_flags(sp);
+
+ if (onstack == 0)
sp = current->sas_ss_sp + current->sas_ss_size;
+ else if (onstack == SS_ONSTACK) {
+ /*
+* If we are on the alternate signal stack and would
+* overflow it, don't return an always-bogus address
+* instead so we will die with SIGSEGV.
+*/
+ if (!likely(on_sig_stack(sp - frame_size)))
+ return (void __user *) -1L;
+ }
}
/* This is the legacy signal stack switching. */
--
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/
[PATCH 2.6.24-rt1] timer:fix build warning in timer.c
Fix the following compile warning without CONFIG_PREEMPT_RT:
kernel/timer.c:937: warning: ‘count_active_rt_tasks’ defined but not used
Signed-off-by: Shi Weihua <[EMAIL PROTECTED]>
---
diff -urpN linux-2.6.24-rt1.orig/kernel/timer.c linux-2.6.24-rt1/kernel/timer.c
--- linux-2.6.24-rt1.orig/kernel/timer.c2008-02-14 17:01:49.0
+0800
+++ linux-2.6.24-rt1/kernel/timer.c 2008-02-14 17:31:10.0 +0800
@@ -930,20 +930,18 @@ static unsigned long count_active_tasks(
#endif
}
+#ifdef CONFIG_PREEMPT_RT
/*
* Nr of active tasks - counted in fixed-point numbers
*/
static unsigned long count_active_rt_tasks(void)
{
-#ifdef CONFIG_PREEMPT_RT
extern unsigned long rt_nr_running(void);
extern unsigned long rt_nr_uninterruptible(void);
return (rt_nr_running() + rt_nr_uninterruptible()) * FIXED_1;
-#else
- return 0;
-#endif
}
+#endif
/*
* Hmm.. Changed this, as the GNU make sources (load.c) seems to
--
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/
Re: [PATCH 1/3] signal(i386): alternative signal stack wraparound occurs
Roland McGrath wrote::
>>> Thank you for your detailed explanation and patch. I tested your
>>> patch, unfortunately it can not stop all kinds of overflow.
>> [...]
>>> So, the patch I posted is still needed
>> thanks, i've picked up your fix for x86.git, for 2.6.25 merging.
>
> I just explained that not all overflows would be caught and that doing so
> would violate the semantics of e.g. longjmp. I don't see how the patch
> you've included now doesn't still have all those problems. I think it's
> wrong.
>
I am sorry, i don't understand how this is related to the semantics of e.g.
longjmp.
But, i am sure my patch solves all overflows. Ingo's patch can't catch the
overflow
which is caught by "int i[1000];" in the handler function.
Do you have more idea for me? Thanks.
Regards
Shi Weihua
By the way, I think Ingo's patch can be improved.
Signed-off-by: Ingo Molnar <[EMAIL PROTECTED]>
Signed-off-by: Thomas Gleixner <[EMAIL PROTECTED]>
Signed-off-by: Shi Weihua <[EMAIL PROTECTED]>
---
--- linux-2.6.24-rc4-git1.orig/arch/x86/kernel/signal_32.c 2007-12-04
12:26:10.0 +0800
+++ linux-2.6.24-rc4-git1/arch/x86/kernel/signal_32.c 2007-12-05
11:13:56.0 +0800
@@ -297,8 +297,17 @@ get_sigframe(struct k_sigaction *ka, str
/* This is the X/Open sanctioned signal stack switching. */
if (ka->sa.sa_flags & SA_ONSTACK) {
- if (sas_ss_flags(esp) == 0)
+ int onstack = sas_ss_flags(esp);
+ if (onstack == 0)
esp = current->sas_ss_sp + current->sas_ss_size;
+ else if(onstack == SS_ONSTACK){
+ /*
+* If we are on the alternate signal stack and would
overflow it, don't.
+* Return an always-bogus address instead so we will
die with SIGSEGV.
+*/
+ if (!likely(on_sig_stack(esp - frame_size)))
+ return (void __user *) -1L;
+ }
}
/* This is the legacy signal stack switching. */
--
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/
[PATCH] IA64 signal : fix missing error checkings
Not all the return value of __copy_from_user and
__put_user is checked.This patch fixed it.
Signed-off-by: Shi Weihua <[EMAIL PROTECTED]>
---
diff -x '*.o*' -urp linux-2.6.24-rc3-git6.orig/arch/ia64/kernel/signal.c
linux-2.6.24-rc3-git6/arch/ia64/kernel/signal.c
--- linux-2.6.24-rc3-git6.orig/arch/ia64/kernel/signal.c2007-12-03
12:04:22.0 +0800
+++ linux-2.6.24-rc3-git6/arch/ia64/kernel/signal.c 2007-12-03
12:50:15.0 +0800
@@ -98,7 +98,7 @@ restore_sigcontext (struct sigcontext __
if ((flags & IA64_SC_FLAG_FPH_VALID) != 0) {
struct ia64_psr *psr = ia64_psr(&scr->pt);
- __copy_from_user(current->thread.fph, &sc->sc_fr[32], 96*16);
+ err |= __copy_from_user(current->thread.fph, &sc->sc_fr[32],
96*16);
psr->mfh = 0; /* drop signal handler's fph contents... */
preempt_disable();
if (psr->dfh)
@@ -244,7 +244,7 @@ static long
setup_sigcontext (struct sigcontext __user *sc, sigset_t *mask, struct
sigscratch *scr)
{
unsigned long flags = 0, ifs, cfm, nat;
- long err;
+ long err = 0;
ifs = scr->pt.cr_ifs;
@@ -257,12 +257,12 @@ setup_sigcontext (struct sigcontext __us
ia64_flush_fph(current);
if ((current->thread.flags & IA64_THREAD_FPH_VALID)) {
flags |= IA64_SC_FLAG_FPH_VALID;
- __copy_to_user(&sc->sc_fr[32], current->thread.fph, 96*16);
+ err = __copy_to_user(&sc->sc_fr[32], current->thread.fph,
96*16);
}
nat = ia64_get_scratch_nat_bits(&scr->pt, scr->scratch_unat);
- err = __put_user(flags, &sc->sc_flags);
+ err |= __put_user(flags, &sc->sc_flags);
err |= __put_user(nat, &sc->sc_nat);
err |= PUT_SIGSET(mask, &sc->sc_mask);
err |= __put_user(cfm, &sc->sc_cfm);
--
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/
Re: Fw: Re: [PATCH 1/3] signal(i386): alternative signal stack wraparound occurs
}
#endif
void segv_handler()
{
#ifdef __i386__
print_esp();
#endif
int *c = NULL;
counter++;
printf("%d\n", counter);
int i[1000];// <- Pay attention here.
*c = 1; // SEGV
}
int main()
{
int *c = NULL;
char *s = malloc(SIGSTKSZ);
stack_t stack;
struct sigaction action;
memset(s, 0, SIGSTKSZ);
stack.ss_sp = s;
stack.ss_flags = 0;
stack.ss_size = SIGSTKSZ;
int error = sigaltstack(&stack, NULL);
if (error) {
printf("Failed to use sigaltstack!\n");
return -1;
}
memset(&action, 0, sizeof(action));
action.sa_handler = segv_handler;
action.sa_flags = SA_ONSTACK | SA_NODEFER;
sigemptyset(&action.sa_mask);
sigaction(SIGSEGV, &action, NULL);
*c = 0; //SEGV
if (!s)
free(s);
return 0;
}
-
So, the patch I posted is still needed
Surely, adding a variable to sched.h is not a good idea.
Could you tell me a better place to store the previous esp?
Here is a new patch rebased on 2.6.24-rc3-git2.
Signed-off-by: Shi Weihua <[EMAIL PROTECTED]>
--- /shiwh-tmp/linux-2.6.24-rc3-git2.orig/arch/x86/kernel/signal_32.c
2007-11-28 09:15:34.0 +0800
+++ /shiwh-tmp/linux-2.6.24-rc3-git2/arch/x86/kernel/signal_32.c
2007-11-28 13:19:13.0 +0800
@@ -297,7 +297,8 @@ get_sigframe(struct k_sigaction *ka, str
/* This is the X/Open sanctioned signal stack switching. */
if (ka->sa.sa_flags & SA_ONSTACK) {
- if (sas_ss_flags(esp) == 0)
+ if (sas_ss_flags(esp) == 0 &&
+ !on_sig_stack(current->pre_ss_sp))
esp = current->sas_ss_sp + current->sas_ss_size;
}
@@ -330,9 +331,15 @@ 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;
+ current->pre_ss_sp = (unsigned long)frame;
+
usig = current_thread_info()->exec_domain
&& current_thread_info()->exec_domain->signal_invmap
&& sig < 32
@@ -422,9 +429,15 @@ 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;
+ current->pre_ss_sp = (unsigned long)frame;
+
usig = current_thread_info()->exec_domain
&& current_thread_info()->exec_domain->signal_invmap
&& sig < 32
--- /shiwh-tmp/linux-2.6.24-rc3-git2.orig/include/linux/sched.h 2007-11-28
09:15:52.0 +0800
+++ /shiwh-tmp/linux-2.6.24-rc3-git2/include/linux/sched.h 2007-11-28
13:20:04.0 +0800
@@ -1059,6 +1059,7 @@ struct task_struct {
struct sigpending pending;
unsigned long sas_ss_sp;
+ unsigned long pre_ss_sp;
size_t sas_ss_size;
int (*notifier)(void *priv);
void *notifier_data;
--- /shiwh-tmp/linux-2.6.24-rc3-git2.orig/kernel/signal.c 2007-11-28
09:15:52.0 +0800
+++ /shiwh-tmp/linux-2.6.24-rc3-git2/kernel/signal.c2007-11-28
13:20:54.0 +0800
@@ -2403,6 +2403,9 @@ do_sigaltstack (const stack_t __user *us
current->sas_ss_sp = (unsigned long) ss_sp;
current->sas_ss_size = ss_size;
+
+ /* reset previous sp */
+ current->pre_ss_sp = 0;
}
if (uoss) {
Thanks,
Shi Weihua
>
>
> Thanks,
> Roland
>
> ---
>
> diff --git a/arch/x86/kernel/signal_32.c b/arch/x86/kernel/signal_32.c
> index d58d455..000 100644
> --- a/arch/x86/kernel/signal_32.c
> +++ b/arch/x86/kernel/signal_32.c
> @@ -295,6 +295,13 @@ get_sigframe(struct k_sigaction *ka, str
> /* Default to using normal stack */
> esp = regs->esp;
>
> + /*
> + * If we are on the alternate signal stack and would overflow it, don't.
> + * Return an always-bogus address instead so we will die with SIGSEGV.
> + */
> + if (on_sig_stack(esp) && !likely(on_sig_stack(esp - frame_size)))
> + return (void __user *) -1L;
> +
> /* This is the X/Open sanctioned signal stack switching. */
> if (ka->sa.sa_flags & SA_ONSTACK) {
> if (sas_ss_flags(esp) == 0)
-
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/
[PATCH RFC][try 2] IA64 signal : remove redundant code in setup_sigcontext()
This patch removes some redundant code in the function setup_sigcontext().
The registers ar.ccv,b7,r14,ar.csd,ar.ssd,r2-r3 and r16-r31 are not restored
in restore_sigcontext() when (flags & IA64_SC_FLAG_IN_SYSCALL) is true.
So we don't need to zero those variables in setup_sigcontext().
Signed-off-by: Shi Weihua <[EMAIL PROTECTED]>
---
diff -urp linux-2.6.24-rc3-git1.orig/arch/ia64/kernel/signal.c
linux-2.6.24-rc3-git1/arch/ia64/kernel/signal.c
--- linux-2.6.24-rc3-git1.orig/arch/ia64/kernel/signal.c2007-11-17
13:16:36.0 +0800
+++ linux-2.6.24-rc3-git1/arch/ia64/kernel/signal.c 2007-11-22
11:02:27.0 +0800
@@ -280,15 +280,7 @@ setup_sigcontext (struct sigcontext __us
err |= __copy_to_user(&sc->sc_gr[15], &scr->pt.r15, 8); /* r15
*/
err |= __put_user(scr->pt.cr_iip + ia64_psr(&scr->pt)->ri, &sc->sc_ip);
- if (flags & IA64_SC_FLAG_IN_SYSCALL) {
- /* Clear scratch registers if the signal interrupted a system
call. */
- err |= __put_user(0, &sc->sc_ar_ccv);
/* ar.ccv */
- err |= __put_user(0, &sc->sc_br[7]);
/* b7 */
- err |= __put_user(0, &sc->sc_gr[14]);
/* r14 */
- err |= __clear_user(&sc->sc_ar25, 2*8); /*
ar.csd & ar.ssd */
- err |= __clear_user(&sc->sc_gr[2], 2*8);
/* r2-r3 */
- err |= __clear_user(&sc->sc_gr[16], 16*8);
/* r16-r31 */
- } else {
+ if (!(flags & IA64_SC_FLAG_IN_SYSCALL)) {
/* Copy scratch regs to sigcontext if the signal didn't
interrupt a syscall. */
err |= __put_user(scr->pt.ar_ccv, &sc->sc_ar_ccv);
/* ar.ccv */
err |= __put_user(scr->pt.b7, &sc->sc_br[7]);
/* b7 */
-
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/
Re: [PATCH 1/3] signal(i386): alternative signal stack wraparound occurs
Hi everyone,
If a process uses alternative signal stack by using sigaltstack(),
then that stack overflows and stack wraparound may occur.
Simple explanation:
The accurate esp order is A,B,C,D,...
But now the esp points to A,B,C and A,B,C... dropping into a recursion.
The upper bug and patch about "alternative signal stack wraparound occurs"
has been contributed here at 10/3.
(subject:[PATCH 0/3] signal: alternative signal stack wraparound occurs)
(Please refer to http://lkml.org/lkml/2007/10/3/41).
Now, I renewed the patch and it can stop wraparound.
Can you give me some advice about storing the previous esp?
Signed-off-by: Shi Weihua <[EMAIL PROTECTED]>
---
diff -urpN linux-2.6.24-rc2.orig/arch/x86/kernel/signal_32.c
linux-2.6.24-rc2/arch/x86/kernel/signal_32.c
--- linux-2.6.24-rc2.orig/arch/x86/kernel/signal_32.c 2007-11-13
14:30:45.0 +0800
+++ linux-2.6.24-rc2/arch/x86/kernel/signal_32.c2007-11-13
14:38:03.0 +0800
@@ -297,7 +297,8 @@ get_sigframe(struct k_sigaction *ka, str
/* This is the X/Open sanctioned signal stack switching. */
if (ka->sa.sa_flags & SA_ONSTACK) {
- if (sas_ss_flags(esp) == 0)
+ if (sas_ss_flags(esp) == 0 &&
+ !on_sig_stack(current->pre_ss_sp))
esp = current->sas_ss_sp + current->sas_ss_size;
}
@@ -330,9 +331,15 @@ 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;
+ current->pre_ss_sp = (unsigned long)frame;
+
usig = current_thread_info()->exec_domain
&& current_thread_info()->exec_domain->signal_invmap
&& sig < 32
diff -urpN linux-2.6.24-rc2.orig/include/linux/sched.h
linux-2.6.24-rc2/include/linux/sched.h
--- linux-2.6.24-rc2.orig/include/linux/sched.h 2007-11-13 14:29:17.0
+0800
+++ linux-2.6.24-rc2/include/linux/sched.h 2007-11-13 14:31:46.0
+0800
@@ -1059,6 +1059,7 @@ struct task_struct {
struct sigpending pending;
unsigned long sas_ss_sp;
+ unsigned long pre_ss_sp;
size_t sas_ss_size;
int (*notifier)(void *priv);
void *notifier_data;
diff -urpN linux-2.6.24-rc2.orig/kernel/signal.c
linux-2.6.24-rc2/kernel/signal.c
--- linux-2.6.24-rc2.orig/kernel/signal.c 2007-11-13 14:29:16.0
+0800
+++ linux-2.6.24-rc2/kernel/signal.c2007-11-13 14:33:00.0 +0800
@@ -2403,6 +2403,9 @@ do_sigaltstack (const stack_t __user *us
current->sas_ss_sp = (unsigned long) ss_sp;
current->sas_ss_size = ss_size;
+
+ /* reset previous sp */
+ current->pre_ss_sp = 0;
}
if (uoss) {
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.c2007-09-26
> 09:44:08.0 +0900
> +++ linux-2.6.23-rc8/arch/i386/kernel/signal.c2007-09-26
> 13:14:25.0 +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;
>
>
> -
> 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/
>
>
>
-
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/
[PATCH] IA64 signal : remove redundant code in setup_sigcontext()
This patch removes some redundant code in the function setup_sigcontext().
The registers ar.ccv,b7,r14,ar.csd,ar.ssd,r2-r3 and r16-r31 are not restored
in restore_sigcontext() when (flags & IA64_SC_FLAG_IN_SYSCALL) is true.
So we don't need to zero those variables in setup_sigcontext().
Signed-off-by: Shi Weihua <[EMAIL PROTECTED]>
---
diff -urp linux-2.6.24-rc2.orig/arch/ia64/kernel/signal.c
linux-2.6.24-rc2/arch/ia64/kernel/signal.c
--- linux-2.6.24-rc2.orig/arch/ia64/kernel/signal.c 2007-11-09
14:38:53.0 +0800
+++ linux-2.6.24-rc2/arch/ia64/kernel/signal.c 2007-11-13 13:30:00.0
+0800
@@ -280,15 +280,7 @@ setup_sigcontext (struct sigcontext __us
err |= __copy_to_user(&sc->sc_gr[15], &scr->pt.r15, 8); /* r15
*/
err |= __put_user(scr->pt.cr_iip + ia64_psr(&scr->pt)->ri, &sc->sc_ip);
- if (flags & IA64_SC_FLAG_IN_SYSCALL) {
- /* Clear scratch registers if the signal interrupted a system
call. */
- err |= __put_user(0, &sc->sc_ar_ccv);
/* ar.ccv */
- err |= __put_user(0, &sc->sc_br[7]);
/* b7 */
- err |= __put_user(0, &sc->sc_gr[14]);
/* r14 */
- err |= __clear_user(&sc->sc_ar25, 2*8); /*
ar.csd & ar.ssd */
- err |= __clear_user(&sc->sc_gr[2], 2*8);
/* r2-r3 */
- err |= __clear_user(&sc->sc_gr[16], 16*8);
/* r16-r31 */
- } else {
+ if (!(flags & IA64_SC_FLAG_IN_SYSCALL)) {
/* Copy scratch regs to sigcontext if the signal didn't
interrupt a syscall. */
err |= __put_user(scr->pt.ar_ccv, &sc->sc_ar_ccv);
/* ar.ccv */
err |= __put_user(scr->pt.b7, &sc->sc_br[7]);
/* b7 */
-
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/
Re: [PATCH 1/3] signal(i386): alternative signal stack wraparound occurs
Mikael Pettersson wrote:: On Thu, 4 Oct 2007 21:47:30 +0900, KAMEZAWA Hiroyuki wrote: On Thu, 04 Oct 2007 21:33:12 +0900 Shi Weihua <[EMAIL PROTECTED]> wrote: KAMEZAWA Hiroyuki wrote:: On Thu, 04 Oct 2007 20:56:14 +0900 Shi Weihua <[EMAIL PROTECTED]> wrote: stack.ss_sp = addr + pagesize; stack.ss_flags = 0; stack.ss_size = pagesize; Here is bad. stack,ss_sp = addr; stack.ss_flags = 0; stack.ss_size = pagesize * 2; [What the test code want to do] addr+pagesize*2 - addr+pagesize -> sigaltstack addr+pagesize - addr -> protected region The code want to catch overflow when esp enter the protected region. You have to protect the top of *registered* sigaltstack. The reason of wraparound is %esp will be set to the bottom of sigaltstack if it is not on sigaltstack area when signaled. What you have to do is protect the top of registerd sigaltstack. If %esp is in the range of registerd sigaltstack at SEGV, wraparound will stop. Exactly right. You mprotect or munmap the end of the altstack, not the area beyond it. So we tell users "Even if you protectted half of mmap's space, but you must to register all space to kernel. " ? The image about my test code's result: No patchPatched ┌───┐ │ │← 1 ┌ ← 3 ← 1 │A ││(wraparound) │ ││ │ │← 2 │ ← 2 │ ││ ├───┤│ │▒▒▒│← 3 ┘ ← 3 │B▒▒│ (caught) │▒protected▒│ │▒▒▒│ │▒▒▒│ └───┘ A+B mmap's space Asigaltstack Bprotectted I agree that if register A+B to kernel, the wraparound will stop. But if register A to kernel, why not kernel do something? Thanks Shi Weihua /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/
Re: [PATCH 1/3] signal(i386): alternative signal stack wraparound occurs
KAMEZAWA Hiroyuki wrote:: On Thu, 04 Oct 2007 20:56:14 +0900 Shi Weihua <[EMAIL PROTECTED]> wrote: stack.ss_sp = addr + pagesize; stack.ss_flags = 0; stack.ss_size = pagesize; Here is bad. stack,ss_sp = addr; stack.ss_flags = 0; stack.ss_size = pagesize * 2; [What the test code want to do] addr+pagesize*2 - addr+pagesize -> sigaltstack addr+pagesize - addr -> protected region The code want to catch overflow when esp enter the protected region. But it failed ... cheers. -Kame - 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/
Re: [PATCH 1/3] signal(i386): alternative signal stack wraparound occurs
KAMEZAWA Hiroyuki wrote::
On Wed, 3 Oct 2007 15:46:32 +0200 (MEST)
Mikael Pettersson <[EMAIL PROTECTED]> wrote:
The proposed kernel signal delivery patch only handles the case
where the /sigframe/ ends up overlapping the end of the altstack.
If the sigframe remains within the altstack boundaries but the
user-space signal handler adds an /ordinary stack frame/ that
moves SP beyond the altstack limit, then the kernel patch solves
nothing and recursive signals will cause altstack wraparound.
On the other hand, the user-space technique of making the lowest
page(s) in the altstack inaccessible handles both cases of overflow.
Hmm, okay. Then, this fix is not enough. I see.
I'll consider how to eduacate users.
Excuse me. What will Mr.Kamezawa educate users? How to use sigaltstack?
Following is about using mmap/mprotect. In the previous mail(just now), I have said the same
thing.Now I say it again in detailed.
Mikael has told us user'd better to use mmap/mprotect. So I tried to use
mmap/mprotect in my test code.
I want to mprotect() the place from mid to low, and hope it stop the overflow.
high
|
| enable to access
|
mid
|
| disable to access
|
low
I hope the kernel catch it when the esp beyond the boundaries(mid) in
user-space.
But the altstack wraparound still occurs.
begin = 0xb7fec000
end = 0xb7fee000
esp = 0xb7fedce0
1
esp = 0xb7fed9e0
2
esp = 0xb7fed6e0
3
esp = 0xb7fedce0 <- wraparound
4
...
Fortunately, when I reuse the patch, wraparound disappeared. Even if I activate the code *1(please
refer to the following test code).
So I think we need the patch, in the same time,we advice the user it's better
to use mmap/mprotect.
---
#include
#include
#include
#include
#include
#include
#define die(msg) do { perror(msg); exit(EXIT_FAILURE); } while (0)
volatile int counter = 0;
#ifdef __i386__
void print_esp()
{
unsigned long esp;
__asm__ __volatile__("movl %%esp, %0":"=g"(esp));
printf("esp = 0x%08lx\n", esp);
}
#endif
static void segv_handler()
{
#ifdef __i386__
print_esp();
#endif
// int i[1000];//*1
int *c = NULL;
counter++;
printf("%d\n", counter);
*c = 1; // SEGV
}
int main()
{
int *c = NULL;
int pagesize;
char *addr;
stack_t stack;
struct sigaction action;
pagesize = sysconf(_SC_PAGE_SIZE);
if (pagesize == -1)
die("sysconf");
addr = mmap(NULL, pagesize * 2, PROT_READ | PROT_WRITE | PROT_EXEC,
MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
if (addr == MAP_FAILED)
die("mmap");
printf("begin = 0x%08lx\n", addr);
printf("end = 0x%08lx\n", addr + pagesize * 2);
if (mprotect(addr, pagesize, PROT_NONE) == -1)
die("mprotect");
stack.ss_sp = addr + pagesize;
stack.ss_flags = 0;
stack.ss_size = pagesize;
int error = sigaltstack(&stack, NULL);
if (error) {
printf("Failed to use sigaltstack!\n");
return -1;
}
memset(&action, 0, sizeof(action));
action.sa_handler = segv_handler;
action.sa_flags = SA_ONSTACK | SA_NODEFER;
sigemptyset(&action.sa_mask);
sigaction(SIGSEGV, &action, NULL);
*c = 0; //SEGV
return 0;
}
---
Any suggestion?
Thanks
Shi Weihua
Thanks,
-Kame
-
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/
Re: [PATCH 1/3] signal(i386): alternative signal stack wraparound occurs
Mikael Pettersson wrote::
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.0 +0900
+++ linux-2.6.23-rc8/arch/i386/kernel/signal.c 2007-09-26 13:14:25.0
+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.
mmap/mprotect can not avoid this kind of wraparound.
Please compile and run the following test code on i386.
The code want to allow process access from high to mid,and not from mid to low.
high
|
|
mid
|
|
low
#include
#include
#include
#include
#include
#include
#define die(msg) do { perror(msg); exit(EXIT_FAILURE); } while (0)
volatile int counter = 0;
#ifdef __i386__
void print_esp()
{
unsigned long esp;
__asm__ __volatile__("movl %%esp, %0":"=g"(esp));
printf("esp = 0x%08lx\n", esp);
}
#endif
static void segv_handler()
{
#ifdef __i386__
print_esp();
#endif
int *c = NULL;
counter++;
printf("%d\n", counter);
*c = 1; // SEGV
}
int main()
{
int *c = NULL;
int pagesize;
char *addr;
stack_t stack;
struct sigaction action;
pagesize = sysconf(_SC_PAGE_SIZE);
if (pagesize == -1) {
die("sysconf");
exit(EXIT_FAILURE);
}
addr = mmap(NULL, pagesize * 2, PROT_READ | PROT_WRITE | PROT_EXEC,
MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
if (addr == MAP_FAILED) {
die("mmap");
exit(EXIT_FAILURE);
}
printf("begin = 0x%08lx\n", addr);
printf("end = 0x%08lx\n", addr + pagesize * 2);
if (mprotect(addr, pagesize, PROT_NONE) == -1) {
die("mprotect");
exit(EXIT_FAILURE);
}
stack.ss_sp = addr + pagesize;
stack.ss_flags = 0;
stack.ss_size = pagesize; //SIGSTKSZ;
int error = sigaltstack(&stack, NULL);
if (error) {
printf("Failed to use sigaltstack!\n");
return -1;
}
memset(&action, 0, sizeof(action));
action.sa_handler = segv_handler;
action.sa_flags = SA_ONSTACK | SA_NODEFER;
sigemptyset(&action.sa_mask);
sigaction(SIGSEGV, &action, NULL);
*c = 0; //SEGV
return 0;
}
Any suggestion?
Thanks
Shi Weihua
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/
[PATCH 1/3] signal(i386): alternative signal stack wraparound occurs
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.0 +0900 +++ linux-2.6.23-rc8/arch/i386/kernel/signal.c 2007-09-26 13:14:25.0 +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; - 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/
[PATCH 2/3] signal(ia64): alternative signal stack wraparound occurs
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 ia64,version is 2.6.23-rc8.
Signed-off-by: Shi Weihua <[EMAIL PROTECTED]>
diff -pur linux-2.6.23-rc8.orig/arch/ia64/ia32/ia32_signal.c
linux-2.6.23-rc8/arch/ia64/ia32/ia32_signal.c
--- linux-2.6.23-rc8.orig/arch/ia64/ia32/ia32_signal.c 2007-07-09
08:32:17.0 +0900
+++ linux-2.6.23-rc8/arch/ia64/ia32/ia32_signal.c 2007-09-26
12:16:09.0 +0900
@@ -766,7 +766,7 @@ get_sigframe (struct k_sigaction *ka, st
/* This is the X/Open sanctioned signal stack switching. */
if (ka->sa.sa_flags & SA_ONSTACK) {
- if (!on_sig_stack(esp))
+ if (sas_ss_flags(esp) == 0)
esp = current->sas_ss_sp + current->sas_ss_size;
}
/* Legacy stack switching not supported */
@@ -787,6 +787,10 @@ setup_frame_ia32 (int sig, struct k_siga
frame = get_sigframe(ka, regs, sizeof(*frame));
+ if ((ka->sa.sa_flags & SA_ONSTACK)
+ && (sas_ss_flags((unsigned long) frame) == 0))
+ goto give_sigsegv;
+
if (!access_ok(VERIFY_WRITE, frame, sizeof(*frame)))
goto give_sigsegv;
@@ -854,6 +858,10 @@ setup_rt_frame_ia32 (int sig, struct k_s
frame = get_sigframe(ka, regs, sizeof(*frame));
+ if ((ka->sa.sa_flags & SA_ONSTACK)
+ && (sas_ss_flags((unsigned long) frame) == 0))
+ goto give_sigsegv;
+
if (!access_ok(VERIFY_WRITE, frame, sizeof(*frame)))
goto give_sigsegv;
diff -pur linux-2.6.23-rc8.orig/arch/ia64/kernel/signal.c
linux-2.6.23-rc8/arch/ia64/kernel/signal.c
--- linux-2.6.23-rc8.orig/arch/ia64/kernel/signal.c 2007-07-09
08:32:17.0 +0900
+++ linux-2.6.23-rc8/arch/ia64/kernel/signal.c 2007-09-26 09:54:10.0
+0900
@@ -362,6 +362,10 @@ setup_frame (int sig, struct k_sigaction
}
frame = (void __user *) ((new_sp - sizeof(*frame)) & -STACK_ALIGN);
+ if ((ka->sa.sa_flags & SA_ONSTACK)
+ && (sas_ss_flags((unsigned long) frame) == 0))
+ return force_sigsegv_info(sig, frame);
+
if (!access_ok(VERIFY_WRITE, frame, sizeof(*frame)))
return force_sigsegv_info(sig, frame);
-
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/
[PATCH 3/3] signal(x86-64): alternative signal stack wraparound occurs
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 x86-64,version is 2.6.23-rc8. Signed-off-by: Shi Weihua <[EMAIL PROTECTED]> diff -pur linux-2.6.23-rc8.orig/arch/x86_64/ia32/ia32_signal.c linux-2.6.23-rc8/arch/x86_64/ia32/ia32_signal.c --- linux-2.6.23-rc8.orig/arch/x86_64/ia32/ia32_signal.c2007-07-09 08:32:17.0 +0900 +++ linux-2.6.23-rc8/arch/x86_64/ia32/ia32_signal.c 2007-09-26 12:09:07.0 +0900 @@ -428,6 +428,10 @@ int ia32_setup_frame(int sig, struct k_s frame = get_sigframe(ka, regs, sizeof(*frame)); + if ((ka->sa.sa_flags & SA_ONSTACK) + && (sas_ss_flags((unsigned long) frame) == 0)) + goto give_sigsegv; + if (!access_ok(VERIFY_WRITE, frame, sizeof(*frame))) goto give_sigsegv; @@ -518,6 +522,10 @@ int ia32_setup_rt_frame(int sig, struct frame = get_sigframe(ka, regs, sizeof(*frame)); + if ((ka->sa.sa_flags & SA_ONSTACK) + && (sas_ss_flags((unsigned long) frame) == 0)) + goto give_sigsegv; + if (!access_ok(VERIFY_WRITE, frame, sizeof(*frame))) goto give_sigsegv; diff -pur linux-2.6.23-rc8.orig/arch/x86_64/kernel/signal.c linux-2.6.23-rc8/arch/x86_64/kernel/signal.c --- linux-2.6.23-rc8.orig/arch/x86_64/kernel/signal.c 2007-09-26 09:44:09.0 +0900 +++ linux-2.6.23-rc8/arch/x86_64/kernel/signal.c2007-09-26 12:09:31.0 +0900 @@ -235,6 +235,10 @@ static int setup_rt_frame(int sig, struc } else frame = get_stack(ka, regs, sizeof(struct rt_sigframe)) - 8; + if ((ka->sa.sa_flags & SA_ONSTACK) + && (sas_ss_flags((unsigned long) frame) == 0)) + goto give_sigsegv; + if (!access_ok(VERIFY_WRITE, frame, sizeof(*frame))) goto give_sigsegv; - 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/
[PATCH 0/3] signal: alternative signal stack wraparound occurs
Hi everyone,
If a process uses alternative signal stack by using sigaltstack(),
then that stack overflows and stack wraparound occurs.
Simple Explanation:
The accurate esp order is A,B,C,D,...
But now the esp points to A,B,C and A,B,C again.
When I tested sigaltstack() and try to kill a same signal SIGSEGV
to the current process,the previous phenomena occurs.
This problem can reproduce by the following code:
---
#include
#include
#include
#include
volatile int counter = 0;
#ifdef __i386__
void print_esp()
{
unsigned long esp;
__asm__ __volatile__("movl %%esp, %0":"=g"(esp));
printf("esp = 0x%08lx\n", esp);
}
#endif
void segv_handler()
{
#ifdef __i386__
print_esp();
#endif
int *c = NULL;
counter++;
printf("%d\n", counter);
*c = 1; // SEGV
}
int main()
{
int *c = NULL;
char *s = malloc(SIGSTKSZ);
stack_t stack;
struct sigaction action;
memset(s, 0, SIGSTKSZ);
stack.ss_sp = s;
stack.ss_flags = 0;
stack.ss_size = SIGSTKSZ;
int error = sigaltstack(&stack, NULL);
if (error) {
printf("Failed to use sigaltstack!\n");
return -1;
}
memset(&action, 0, sizeof(action));
action.sa_handler = segv_handler;
action.sa_flags = SA_ONSTACK | SA_NODEFER;
sigemptyset(&action.sa_mask);
sigaction(SIGSEGV, &action, NULL);
*c = 0; //SEGV
if (!s)
free(s);
return 0;
}
---
The result(for i386) is as follows:
---
$ ./testpro
esp = 0x0804bcf0
1
esp = 0x0804b9f0
2
esp = 0x0804b6f0
3
esp = 0x0804b3f0
4
esp = 0x0804b0f0
5
esp = 0x0804adf0
6
esp = 0x0804aaf0
7
esp = 0x0804a7f0
8
esp = 0x0804a4f0
9
esp = 0x0804a1f0
10
esp = 0x08049ef0
11
esp = 0x0804bcf0
12 # <- wraparound occurs!
# <- the 12nd output is same as 1st.
---
The kernel doesn't stop the wraparound occuring, so I think this is a bug.
In my patches,some check code is added.
These code checks the signal frame is on alternative signal stack or not.
If signal frame isn't on the stack, an error process mechanism will be
awoken(e.g."goto give_sigsegv" ).
[PATCH 1/3] signal(i386): alternative signal stack wraparound occurs
[PATCH 2/3] signal(ia64): alternative signal stack wraparound occurs
[PATCH 3/3] signal(x86-64): alternative signal stack wraparound occurs
My patch resolved this wraparound's problem, but if I use the following patch
on the previous test code,the wraparound still occurs.
---
--- testpro.c.orig 2007-09-26 11:11:45.0 +0900
+++ testpro.c 2007-09-26 11:12:46.0 +0900
@@ -21,6 +21,8 @@ void segv_handler()
print_esp();
#endif
+ int i[1000];
+
int *c = NULL;
counter++;
printf("%d\n", counter);
---
I think the "int i[1000];" make the signal frame not to be checked by the added
code in my patch.
But I don't know how to avoid it.
Do you have any idea?
Regards,
Shi Weihua
-
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/
