[PATCH] x86/nSVM: Don't zero the l1 guest's N_CR3 on #VMEXIT

Stephen Cheng posted 1 patch 3 days, 13 hours ago
xen/arch/x86/hvm/svm/nestedsvm.c | 9 ++++-----
1 file changed, 4 insertions(+), 5 deletions(-)
[PATCH] x86/nSVM: Don't zero the l1 guest's N_CR3 on #VMEXIT
Posted by Stephen Cheng 3 days, 13 hours ago
Xen's emulated #VMEXIT writes a VMCB field that a real #VMEXIT leaves alone.
nsvm_vmcb_prepare4vmexit() zeroes ns_vmcb->_h_cr3 when the l1 guest runs its
l2 guest with nested paging off.  Hardware writes back guest state and the
exit-information fields (AMD APM vol 2 rev 3.44 section 15.6); N_CR3 is
neither, and section 15.25.4 says so by name: "nCR3 is not saved back into
the VMCB".  An l1 guest that sets N_CR3 once and reuses the VMCB finds it
zeroed.

The comment defending it - the guest "is not allowed to set" h_cr3,
"otherwise (security hole!)" - is wrong.  ns_vmcb is a live mapping of l1
guest memory, so the guest can write the field again before the next VMRUN.
Drop both assignments and that comment.

Assisted-by: Claude:claude-opus-5
Signed-off-by: Stephen Cheng <stephen.cheng@citrix.com>
---
 xen/arch/x86/hvm/svm/nestedsvm.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/xen/arch/x86/hvm/svm/nestedsvm.c b/xen/arch/x86/hvm/svm/nestedsvm.c
index a8b15d6eae..54c62d1474 100644
--- a/xen/arch/x86/hvm/svm/nestedsvm.c
+++ b/xen/arch/x86/hvm/svm/nestedsvm.c
@@ -1023,7 +1023,10 @@ nsvm_vmcb_prepare4vmexit(struct vcpu *v, struct cpu_user_regs *regs)
 
     ns_vmcb->event_inj.raw = 0;
 
-    /* Nested paging mode */
+    /*
+     * Nested paging mode.  ns_vmcb->_h_cr3 is left alone: hardware does not
+     * save N_CR3 back into the VMCB on #VMEXIT.
+     */
     if ( nestedhvm_paging_mode_hap(v) )
     {
         /* host nested paging + guest nested paging. */
@@ -1037,9 +1040,6 @@ nsvm_vmcb_prepare4vmexit(struct vcpu *v, struct cpu_user_regs *regs)
     {
         /* host nested paging + guest shadow paging. */
         vmcb_set_np(ns_vmcb, false);
-        /* Throw h_cr3 away. Guest is not allowed to set it or
-         * it can break out, otherwise (security hole!) */
-        ns_vmcb->_h_cr3 = 0x0;
         /* Stop intercepting #PF (already done above
          * by restoring cached intercepts). */
         ns_vmcb->_cr3 = n2vmcb->_cr3;
@@ -1048,7 +1048,6 @@ nsvm_vmcb_prepare4vmexit(struct vcpu *v, struct cpu_user_regs *regs)
     {
         /* host shadow paging + guest shadow paging. */
         vmcb_set_np(ns_vmcb, false);
-        ns_vmcb->_h_cr3 = 0x0;
         /* The vmcb->_cr3 is the shadowed cr3. The original
          * unshadowed guest cr3 is kept in ns_vmcb->_cr3,
          * hence we keep the ns_vmcb->_cr3 value. */
-- 
2.49.0
Re: [PATCH] x86/nSVM: Don't zero the l1 guest's N_CR3 on #VMEXIT
Posted by Stephen Cheng 3 days, 12 hours ago
Please drop this one.  Ross posted a fix for the same thing three days
before mine, which I missed:

  https://lore.kernel.org/xen-devel/20260918125205.374168-1-ross.lagerwall@citrix.com/

His patch also drops the NP_ENABLE writes, so it covers strictly more
than mine.  Sorry for the noise.

Worth mentioning: I have an XTF test that catches this issue.

Stephen