[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

  • To: Jan Beulich <jbeulich@xxxxxxxxxx>
  • From: Keir Fraser <Keir.Fraser@xxxxxxxxxxxx>
  • Date: Wed, 09 May 2007 09:34:31 +0100
  • Cc: xen-devel@xxxxxxxxxxxxxxxxxxx
  • Delivery-date: Wed, 09 May 2007 01:31:08 -0700
  • List-id: Xen developer discussion <xen-devel.lists.xensource.com>
  • Thread-index: AceSFN0cG5/1mv4IEdu9MQAWy6hiGQ==
  • Thread-topic: [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().

> 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'

 -- 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



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