[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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



>>> Keir Fraser <Keir.Fraser@xxxxxxxxxxxx> 09.05.07 09:51 >>>
>On 9/5/07 08:25, "Jan Beulich" <jbeulich@xxxxxxxxxx> wrote:
>
>> What is the reason for save_svm_cpu_user_regs() explicitly saving rax?
>> Without this, the function could be eliminated, with its single use replaced
>> by a call to svm_store_cpu_guest_regs().
>
>It would be great if we can get away with just moving save/restore of rAX
>into svm_{load,store}_cpu_guest_regs(), kill save_svm_cpu_user_regs()
>completely, and get rid of the call at the top of the vmexit handler.
>There's no equivalent call at the top of the VMX vmexit handler: all the
>common HVM code will explicitly svm_store_cpu_guest_regs() before depending
>on GPR state.

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 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:
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)...

Jan


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


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.