Some time ago, the i386 kernel was patched to support user code that
has signal handlers marked with __attribute__((regparm(3))) or compiled
with the -mregparm=3 gcc option. This is useful for klibc at least.
The code wasn't quite right, and it never got ported to x86_64.
For i386, the non-RT frame is wrong. It was using the raw sig value
instead of the translated value, and did not pass the semi-documented
extra parameters.
For x86-64, the regparm 3 calling convention was simply missing.
This patch should do the job, provided I'm right in guessing that a
struct passed by value is really passed as a pointer in this case.
(the non-RT signal handler's second arg is a pass-by-value struct)
Signed-off-by: Albert Cahalan <[email protected]>
---
I'm sending this inline for review, and attached because
the whitespace will surely be mangled.
diff -Naurd old/arch/i386/kernel/signal.c new/arch/i386/kernel/signal.c
--- old/arch/i386/kernel/signal.c 2006-09-14 03:49:30.000000000 -0400
+++ new/arch/i386/kernel/signal.c 2006-09-14 03:52:54.000000000 -0400
@@ -375,9 +375,9 @@
/* Set up registers for signal handler */
regs->esp = (unsigned long) frame;
regs->eip = (unsigned long) ka->sa.sa_handler;
- regs->eax = (unsigned long) sig;
- regs->edx = (unsigned long) 0;
- regs->ecx = (unsigned long) 0;
+ regs->eax = (unsigned long) usig;
+ regs->edx = (unsigned long) &frame->sc;
+ regs->ecx = (unsigned long) &frame->fpstate;
set_fs(USER_DS);
regs->xds = __USER_DS;
diff -Naurd old/arch/x86_64/ia32/ia32_signal.c
new/arch/x86_64/ia32/ia32_signal.c
--- old/arch/x86_64/ia32/ia32_signal.c 2006-09-14 03:21:21.000000000 -0400
+++ new/arch/x86_64/ia32/ia32_signal.c 2006-09-14 03:47:32.000000000 -0400
@@ -433,6 +433,7 @@
{
struct sigframe __user *frame;
int err = 0;
+ int usig;
frame = get_sigframe(ka, regs, sizeof(*frame));
@@ -441,12 +442,10 @@
{
struct exec_domain *ed = current_thread_info()->exec_domain;
- err |= __put_user((ed
- && ed->signal_invmap
- && sig < 32
- ? ed->signal_invmap[sig]
- : sig),
- &frame->sig);
+ usig = ed && ed->signal_invmap && sig < 32
+ ? ed->signal_invmap[sig]
+ : sig;
+ err |= __put_user(usig,&frame->sig);
}
if (err)
goto give_sigsegv;
@@ -493,6 +492,9 @@
/* Set up registers for signal handler */
regs->rsp = (unsigned long) frame;
regs->rip = (unsigned long) ka->sa.sa_handler;
+ regs->rax = (unsigned long) usig;
+ regs->rdx = (unsigned long) &frame->sc;
+ regs->rcx = (unsigned long) &frame->fpstate;
asm volatile("movl %0,%%ds" :: "r" (__USER32_DS));
asm volatile("movl %0,%%es" :: "r" (__USER32_DS));
@@ -522,6 +524,7 @@
{
struct rt_sigframe __user *frame;
int err = 0;
+ int usig;
frame = get_sigframe(ka, regs, sizeof(*frame));
@@ -530,12 +533,10 @@
{
struct exec_domain *ed = current_thread_info()->exec_domain;
- err |= __put_user((ed
- && ed->signal_invmap
- && sig < 32
- ? ed->signal_invmap[sig]
- : sig),
- &frame->sig);
+ usig = ed && ed->signal_invmap && sig < 32
+ ? ed->signal_invmap[sig]
+ : sig;
+ err |= __put_user(usig,&frame->sig);
}
err |= __put_user(ptr_to_compat(&frame->info), &frame->pinfo);
err |= __put_user(ptr_to_compat(&frame->uc), &frame->puc);
@@ -589,6 +590,9 @@
/* Set up registers for signal handler */
regs->rsp = (unsigned long) frame;
regs->rip = (unsigned long) ka->sa.sa_handler;
+ regs->rax = (unsigned long) usig;
+ regs->rdx = (unsigned long) &frame->info;
+ regs->rcx = (unsigned long) &frame->uc;
asm volatile("movl %0,%%ds" :: "r" (__USER32_DS));
asm volatile("movl %0,%%es" :: "r" (__USER32_DS));
On Thursday 14 September 2006 10:34, Albert Cahalan wrote:
> For i386, the non-RT frame is wrong. It was using the raw sig value
> instead of the translated value, and did not pass the semi-documented
> extra parameters.
The translation is not needed because x86-64 doesn't support iBCS at all
and afaik it was only used for that.
>
> For x86-64, the regparm 3 calling convention was simply missing.
Ok that should be added.
Can you please send a single patch that just does this?
-Andi
On 9/14/06, Andi Kleen <[email protected]> wrote:
> On Thursday 14 September 2006 10:34, Albert Cahalan wrote:
>
> > For i386, the non-RT frame is wrong. It was using the raw sig value
> > instead of the translated value, and did not pass the semi-documented
> > extra parameters.
>
> The translation is not needed because x86-64 doesn't support iBCS at all
> and afaik it was only used for that.
I figured, but you already have part of the code it seems.
(messing around with current_thread_info()->exec_domain
to get ->signal_invmap)
I guess that should be deleted then?
Currently you remap signals. Whatever you do this for
regparm(0) should also be done for regparm(3).
> I guess that should be deleted then?
Yes. I will delete it right now. Thanks for the notice.
> Currently you remap signals. Whatever you do this for
> regparm(0) should also be done for regparm(3).
Not sure I parse you here. You're asking how to fix the regparm(3)
case?
-Andi
On 9/14/06, Andi Kleen <[email protected]> wrote:
>
> > I guess that should be deleted then?
>
> Yes. I will delete it right now. Thanks for the notice.
Er, OK. This means I can't patch it without conflict.
Mind just adding the six lines of code needed for
support of regparm(3) apps?
> > Currently you remap signals. Whatever you do this for
> > regparm(0) should also be done for regparm(3).
>
> Not sure I parse you here. You're asking how to fix the regparm(3)
> case?
No. I'd thought that the two cases should match.
The regparm(3) case should remap signals if and only if
the regparm(0) case remaps signals. Perhaps this
is not correct if the remapping is not needed for
native Linux apps; I doubt iBCS stuff would ever be
needing regparm(3) support.
Since you plan to delete the remapping cruft from
the regparm(0) case, then obviously it should not
be added to the regparm(3) case.
Albert Cahalan wrote:
>
>> > Currently you remap signals. Whatever you do this for
>> > regparm(0) should also be done for regparm(3).
>>
>> Not sure I parse you here. You're asking how to fix the regparm(3)
>> case?
>
> No. I'd thought that the two cases should match.
> The regparm(3) case should remap signals if and only if
> the regparm(0) case remaps signals. Perhaps this
> is not correct if the remapping is not needed for
> native Linux apps; I doubt iBCS stuff would ever be
> needing regparm(3) support.
>
The two should definitely match, though. Otherwise, life will be confusing.
> Since you plan to delete the remapping cruft from
> the regparm(0) case, then obviously it should not
> be added to the regparm(3) case.
Indeed.
-hpa