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

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <49614F62.5070006@kernel.org>
Date:	Sun, 04 Jan 2009 16:08:02 -0800
From:	"H. Peter Anvin" <hpa@...nel.org>
To:	Andi Kleen <andi@...stfloor.org>
CC:	akpm@...ux-foundation.org, x86@...nel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH] [4/5] Use asm stubs for 32bit sigreturn codes

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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/