Re: [PATCH] x86_64: another fix for canonical RIPs during signal handling

[Willy Tarreau <w@1wt.eu>]

On Thu, Jun 22, 2006 at 11:26:22PM +0200, Andi Kleen wrote:
> On Thursday 22 June 2006 23:06, Willy Tarreau wrote:
> > 
> > I've been reported by the PaX Team that the following fix left a
> > small hole :
> > 
> > [PATCH] Always check that RIPs are canonical during signal handling
> > 
> > +	if (regs->rip >= TASK_SIZE && regs->rip < VSYSCALL_START) {
> > +		regs->rip = 0;
> > +		return -EFAULT;
> > +	}
> > 
> > ...
> > 
> > +	if (regs->rip >= TASK_SIZE) {
> > +		if (sig == SIGSEGV)
> > +			ka->sa.sa_handler = SIG_DFL;
> > +		regs->rip = 0;
> > +	}
> > 
> > "the wrong part is regs->rip=0, i guess the intention was to cause a
> >  SIGSEGV upon returning to userland, but 0 is a valid userland address,
> >  an application may very well have something mapped there. the correct
> >  value would be ~0UL as it's guaranteed to fault on linux."
> > 
> > This explanation makes sense, so here's the patch. Andi, would you please
> > review and confirm ? Thanks in advance.
> 
> I don't think it's a real problem.
> 
> The patch is not wrong, but also doesn't fix something that needs
> to be fixed.

What I understand from this is if code is mapped at 0 (eg by mmap(PROT_EXEC)),
it would get executed instead of the program being killed. Although I don't
see how this could be exploited to gain any privileges, I wonder if it can
cause a process to loop indefinitely instead of being killed or nasty things
like this. May be this is a stupid analysis from me, so I hope that PaX Team
will have more precise info.

> -Andi

Regards,
Willy