RE: [Xen-devel] svm: save_svm_cpu_user_regs vs. svm_store_cpu_guest_regs

["Petersson, Mats" <Mats.Petersson@xxxxxxx>]

> -----Original Message-----
> From: xen-devel-bounces@xxxxxxxxxxxxxxxxxxx 
> [mailto:xen-devel-bounces@xxxxxxxxxxxxxxxxxxx] On Behalf Of 
> Keir Fraser
> Sent: 09 May 2007 09:35
> To: Jan Beulich
> Cc: xen-devel@xxxxxxxxxxxxxxxxxxx
> Subject: Re: [Xen-devel] svm: save_svm_cpu_user_regs vs. 
> svm_store_cpu_guest_regs
> 
> On 9/5/07 09:11, "Jan Beulich" <jbeulich@xxxxxxxxxx> wrote:
> 
> > It's not really GPR state that matters here (GPRs are saved 
> in the respective
> > exit.S files), which is why I wondered about the need for 
> saving RAX.
> 
> The rAX that is saved on the stack in exits.S is not the 
> guest rAX. It's a
> bogus value which gets overwritten by save_svm_cpu_user_regs().
> 
> The fact that the rAX value then gets stuffed back into VMCB 
> in exits.S on
> vmentry is pretty disgusting. We should move the rAX loading 
> on vmexit into
> exits.S as well for symmetry, and kill save_svm_cpu_user_regs().

Yes, that's my fault. I got tired of having a million checks of the type
"if (reg == RAX) ... vmcb->rax ... ; else ... regs->something ...;" in
the svm-code to deal with the fact that the RAX register isn't part of
the normally saved on the stack registers. 

I agree that is should be symmetrical tho'. 

> 
> > The point
> > here is that CS:RIP, RFLAGS, and SS:RSP may need to be 
> stored, and the fact
> > that VMX doesn't do so doesn't mean it can be freely 
> removed from SVM code:
> 
> Okay then, more precisely: modulo bugs it can be removed. Or 
> we load/save
> all VMCB state into the regs structure in exits.S, increasing latency
> slightly for all vmexits but avoiding any risk of 
> inconsistent 'struct regs'
> state.

Agreed. 

--
Mats
> 
>  -- Keir
> 
> > If I'm seeing things right (I didn't check this on 
> hardware, yet), hvm
> > hypercalls
> > are currently having a security hole on VMX in that ring 3 
> code can possibly
> > invoke them and/or prevent ring 0 from invoking them - VMX 
> code doesn't seem
> > to save the CS selector anywhere, but the ring_3() test 
> depends on that (so
> > generally appears to operate on stale data, i.e. whatever 
> was saved on the
> > stack the last time vmx_store_cpu_guest_regs() was 
> executed. If I wasn't
> > buried in hunting bugs for SLE10 SP1, I would have 
> confirmed this already and
> > sent a patch if necessary (along with quite a few more 
> ones, more or less all
> > addressing 32on64/hvm weakness)...
> 
> 
> 
> _______________________________________________
> Xen-devel mailing list
> Xen-devel@xxxxxxxxxxxxxxxxxxx
> http://lists.xensource.com/xen-devel
> 
> 
> 



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel