Re: [PATCH 4/5] x86-64: Replace vsyscall gettimeofday fallback with int 0xcc

[Ingo Molnar <mingo@elte.hu>]

* Andrew Lutomirski <luto@mit.edu> wrote:

> > Ok, i suspect you marked it 0xCC because that's the INT3 instruction
> > - not very useful for exploits?
> 
> Exactly.
> 
> The comments in irq_vectors.h make it sound like vectors 0x81..0xed 
> are used for device interrupts but AFAICT it's only 0x20..0x39 that 
> are used, so the precise choice of vector doesn't matter that much.

No, we use almost all of the vector space for device interrupts. Why 
do you think only 0x20..0x39 is used?

> > A not-so-nit: i'd not limit this message to unhandled signals 
> > alone. An attacker could install a SIGSEGV handler, send a 
> > SIGSEGV and attempt the exploit right then - he'll get a free 
> > attempt with no logging performed, right?.
> 
> I think if an exploit can call sigaction, then we've already lost. 

Yes, indeed. In theory an app could be catching SIGSEGV and we could 
have an exploit there. But that's pretty theoretical ...

> But I can still make the change.

If you did it to not repeat the message then i think the 
printk_ratelimit() is more than enough. force_sig() will be able to 
sort out repeat signals just fine.

> >> +     local_irq_disable();
> >> +     return;
> >> +}
> >
> > Nit: no need for a 'return;' at the end of a void function.
> 
> :)
> 
> That pointless "return" statement was to hide the fact that the
> local_irq_enable wasn't correctly matched.

indeed. I noticed the do_exit() local_irq_enable() assymetry which is 
harmless (we never return), but missed the force_sig() one that isn't 
so harmless.

> I'm changing this code a fair bit in preparation for the extra 
> bonus patch to defang vsyscalls even more by trapping all of them.

ok.

Thanks,

	Ingo