Re: [PATCH] [4/5] Use asm stubs for 32bit sigreturn codes

H. Peter Anvin
2009-01-05T00:08:31Z

Andi Kleen wrote:
> 32bit sigreturn/rt_sigreturn currently uses some magic casting
> with assumptions about the stack layout
> to simulate call by reference for the pt_regs structure on the
> stack. This is fragile, non standard and according to reports
> breaks with LLVM at least. I suppose it could break in
> future gcc versions too when their stack layout changes.
>
> So instead of having this magic in C do it in small
> assembler stubs similar to what x86-64 does (which
> always passes a pointer to pt_regs for such functions)
>
> This also leads to cleaner code.
>

It's wrong, however:

#define asmlinkage CPP_ASMLINKAGE __attribute__((regparm(0)))

> +ENTRY(stub_sigreturn)
> + movl %esp,%eax
> + jmp sys_sigreturn
> +END(stub_sigreturn)

This assumes regparm=3, i.e !asmlinkage.

> -asmlinkage unsigned long sys_sigreturn(unsigned long __unused)
> +asmlinkage unsigned long sys_sigreturn(struct pt_regs *regs)
> {
> struct sigframe __user *frame;
> - struct pt_regs *regs;
> unsigned long ax;
> sigset_t set;
>
> - regs = (struct pt_regs *) &__unused;
> frame = (struct sigframe __user *)(regs->sp - 8);
>
> if (!access_ok(VERIFY_READ, frame, sizeof(*frame)))
> @@ -212,7 +210,7 @@
> return 0;
> }

I think we already have a patch in the tree that changes this to:

asmlinkage unsigned long sys_sigreturn(struct pt_regs regs)

... although doing it as a pointer is better, but requires a change to
the asmlinkage thing.

Getting rid of __attribute__((regparm(0))) for asmlinkage on i386 would
definitely be good, but is a bigger thing than only these couple of
functions.

-hpa