Patchwork [2/2] do_rt_sigreturn provide the target sp

login
register
mail settings
Submitter Bob Picco
Date Sept. 27, 2012, 3:36 p.m.
Message ID <1348760205-26101-3-git-send-email-bpicco@meloft.net>
Download mbox | patch
Permalink /patch/187405/
State Changes Requested
Delegated to: David Miller
Headers show

Comments

Bob Picco - Sept. 27, 2012, 3:36 p.m.
From: bob picco <bpicco@meloft.net>

do_sigaltstack expects the stack we are returning to. An issue will manifest
with an alternate stack. That is, a sigaltstack followed by a sigaction
(aka. rt_sigaction) with SA_ONSTACK for flags will sigsegv. do_sigaltstack
returns -EPERM having found on_sig_stack to be true and a sigsegv results.

Let's teach do_rt_sigreturn to provide do_sigaltstack with our target stack.

Signed-off-by: Bob Picco <bpicco@meloft.net>
cc: stable@vger.kernel.org
---
 arch/sparc/kernel/signal_64.c |    2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
David Miller - Sept. 28, 2012, 2:15 a.m.
From: Bob Picco <bpicco@meloft.net>
Date: Thu, 27 Sep 2012 11:36:45 -0400

> do_sigaltstack expects the stack we are returning to. An issue will manifest
> with an alternate stack. That is, a sigaltstack followed by a sigaction
> (aka. rt_sigaction) with SA_ONSTACK for flags will sigsegv. do_sigaltstack
> returns -EPERM having found on_sig_stack to be true and a sigsegv results.
> 
> Let's teach do_rt_sigreturn to provide do_sigaltstack with our target stack.

"sf" is our target stack frame in the stack frame pointed to by
UREG_FP.

You really haven't described the real reason why this doesn't work
as-is, and therefore your commit message is inadequate and made me do
a lot of work I shouldn't have to do.

Your commit message implies that we are not passing in something
related to the stack frame, but we actually are.

And if this location is wrong, why didn't you update all of the other
do_sigaltstack() callers in the sparc port.  By your description they
should be wrong too.

But those cases don't matter like this one does.  This case here is
special, but you haven't described what is so unique about this case
which causes only it to need special handling.

Do you know the real reason why it fails without your change?
--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Bob Picco - Sept. 28, 2012, 9:56 a.m.
Hi:
David Miller wrote:	[Thu Sep 27 2012, 10:15:54PM EDT]
> From: Bob Picco <bpicco@meloft.net>
> Date: Thu, 27 Sep 2012 11:36:45 -0400
> 
> > do_sigaltstack expects the stack we are returning to. An issue will manifest
> > with an alternate stack. That is, a sigaltstack followed by a sigaction
> > (aka. rt_sigaction) with SA_ONSTACK for flags will sigsegv. do_sigaltstack
> > returns -EPERM having found on_sig_stack to be true and a sigsegv results.
> > 
> > Let's teach do_rt_sigreturn to provide do_sigaltstack with our target stack.
> 
> "sf" is our target stack frame in the stack frame pointed to by
> UREG_FP.
> 
> You really haven't described the real reason why this doesn't work
> as-is, and therefore your commit message is inadequate and made me do
> a lot of work I shouldn't have to do.
> 
> Your commit message implies that we are not passing in something
> related to the stack frame, but we actually are.
> 
> And if this location is wrong, why didn't you update all of the other
> do_sigaltstack() callers in the sparc port.  By your description they
> should be wrong too.
> 
> But those cases don't matter like this one does.  This case here is
> special, but you haven't described what is so unique about this case
> which causes only it to need special handling.
> 
> Do you know the real reason why it fails without your change?
Sorry about the deficient change log. Won't happen again! It was a
couple months ago when I had full context and was perhaps wrong. It was a
combination of glibc and kernel investigation/debug and very subtle. I'll
review and have a thorough explanation in the change log soon.

thanx,

bob
--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
David Miller - Sept. 28, 2012, 5:40 p.m.
From: Bob Picco <bpicco@meloft.net>
Date: Fri, 28 Sep 2012 05:56:30 -0400

> Sorry about the deficient change log. Won't happen again! It was a
> couple months ago when I had full context and was perhaps wrong. It was a
> combination of glibc and kernel investigation/debug and very subtle. I'll
> review and have a thorough explanation in the change log soon.

I thought you actually knew offhand. :-/

It's because of the STACK_BIAS, that's what's unique in this case.
--
To unsubscribe from this list: send the line "unsubscribe sparclinux" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

diff --git a/arch/sparc/kernel/signal_64.c b/arch/sparc/kernel/signal_64.c
index 1f905f6..f572586 100644
--- a/arch/sparc/kernel/signal_64.c
+++ b/arch/sparc/kernel/signal_64.c
@@ -295,7 +295,7 @@  void do_rt_sigreturn(struct pt_regs *regs)
 		err |= restore_fpu_state(regs, fpu_save);
 
 	err |= __copy_from_user(&set, &sf->mask, sizeof(sigset_t));
-	err |= do_sigaltstack(&sf->stack, NULL, (unsigned long)sf);
+	err |= do_sigaltstack(&sf->stack, NULL, regs->u_regs[UREG_FP]);
 
 	if (err)
 		goto segv;