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

["H. Peter Anvin" <hpa@kernel.org>]

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