Re: [patch] kvm: make cr3 loading more robust

[Ingo Molnar <mingo-X9Un+BFzKDI@public.gmane.org>]

* Avi Kivity <avi-atKUWr5tajBWk0Htik3J/w@public.gmane.org> wrote:

> Ingo Molnar wrote:
> >* Avi Kivity <avi-atKUWr5tajBWk0Htik3J/w@public.gmane.org> wrote:
> >
> >  
> >>>rmap_write_protect() has a BUG_ON() if a physical address is not 
> >>>found the the memslot. But this is a possible scenario if a buggy 
> >>>guest OS loads an invalid or corrupted cr3 value. So exit more 
> >>>gracefully.
> >>>      
> >>I think a better solution is to detect the invalid cr3 in new_cr3(). 
> >>That way the entire chain of error returns is avoided.
> >>    
> >
> >i'm wondering what the right semantics would be though. Kill the VM 
> >context?
> >
> >on a real CPU an invalid cr3 does nothing explicitly (besides being a 
> >sure way to get a triple fault) - physical memory is always accessible, 
> >non-present physical memory at most generates an #MCE, but is typically 
> >just returning 0xff, right? So perhaps keep a non-mapped page filled 
> >with 0xff and map non-existent RAM to that? But then this 'RAM' needs to 
> >be made non-writable.
> 
> That's a good solution.  I don't see why it has to be made 
> non-writable -- it has undefined content, and any old value will do.  
> We have (or maybe had) something like that somewhere.

it should always return 0xff content because that's how real hardware 
behaves. It's essentially ROM-alike, with 0xff content. Writes are 
ignored. (last i checked)

also having it writable means it's an information leak as we want to 
share this page amongst guests, etc. Then explicitly faulting the guest 
would be alot cleaner.

> > Looks a bit messy. Cleanest would be to kill the VM context (without 
> >injecting any fault into the guest), but that would then require an 
> >error return chain from set_cr3() ...
> 
> set_cr3() is fairly close to the top.  I'm mostly worried about 
> keeping the innards of the mmu clean.

ok, agreed.

> An alternative is to add a flag to the vcpu which would be examined on 
> entry (vcpu->triple_faulted).

well, the triple fault isnt really explicit behavior of the cr3 loading, 
it is "just" a side-effect of having an all-0xff piece of physical 
memory holding the CPU's page tables. Such a cr3 can be loaded fine, but 
the next instruction fetched will be 0xff 0xff, which should be an 
undefined opcode. The resulting fault will try to execute based off an 
invalid IDT so we get a double fault, which then also tries to execute 
0xff 0xff (if the IDT entry didnt generate a #GPF beforehand, due to an 
invalid segment descriptor) so it results in a triple fault. Does VMX 
report triple faults?

	Ingo

-------------------------------------------------------------------------
Take Surveys. Earn Cash. Influence the Future of IT
Join SourceForge.net's Techsay panel and you'll get the chance to share your
opinions on IT & business topics through brief surveys - and earn cash
http://www.techsay.com/default.php?page=join.php&p=sourceforge&CID=DEVDEV