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

[PATCH] x86/entry: Fix VERW offsets in restore_all_xen()



As explained in the comment partially in context, and contrary to what was
said in commit 8af337dfb8e2 ("x86/entry: Use POP_GPRS and remove
RESTORE_ALL"), the restore_all_xen() path cannot use default offsets for
SPEC_CTRL_COND_VERW.

Getting this wrong is surprisingly benign.  VERW doesn't fault for any limit
or descriptor reasons.

However, in SVM vCPU context LDTR is the guest's not Xen's.  When the segment
selector VERW uses happens to be an LDT selector, the CPU accesses the guest
LDTR in Xen context:

  (XEN) ----[ Xen-4.23.0  x86_64  debug=y  Not tainted ]----
  (XEN) CPU:    14
  (XEN) RIP:    e008:[<ffff82d0402007f2>] 
x86_64/entry.S#restore_all_xen+0x72/0x80
  (XEN) RFLAGS: 0000000000010002   CONTEXT: hypervisor (d3v1)
  ...
  (XEN) Xen code around <ffff82d0402007f2> 
(x86_64/entry.S#restore_all_xen+0x72/0x80):
  (XEN)  5f f6 44 24 7c 08 74 05 <0f> 00 6c 24 44 48 83 c4 08 48 cf 0f 1f 00 fc 
0f
  ...
  (XEN) Xen call trace:
  (XEN)    [<ffff82d0402007f2>] R x86_64/entry.S#restore_all_xen+0x72/0x80
  (XEN)    [<ffff82d0402e79f6>] F nestedhap_fix_p2m+0x5f/0xc9
  (XEN)    [<ffff82d0402e7c8f>] F nestedhvm_hap_nested_page_fault+0x11e/0x22e
  (XEN)    [<ffff82d0402cd59d>] F hvm_hap_nested_page_fault+0x1b8/0x5d2
  (XEN)    [<ffff82d0402ad7de>] F svm_vmexit_handler+0xbe9/0x18b3
  (XEN)    [<ffff82d040202542>] F svm_asm_do_resume+0x162/0x172
  (XEN)
  (XEN) Pagetable walk from 000000000000fff8:
  (XEN)  L4[0x000] = 0000000000000000 ffffffffffffffff
  (XEN)
  (XEN) ****************************************
  (XEN) Panic on CPU 14:
  (XEN) FATAL PAGE FAULT
  (XEN) [error_code=0000]
  (XEN) Faulting linear address: 000000000000fff8
  (XEN) ****************************************

In this case, the guest's LDT obviously has a base of 0 and limit of 0xffff
for the segmentation checks to pass and a memory access to be attempted.

Fixes: 8af337dfb8e2 ("x86/entry: Use POP_GPRS and remove RESTORE_ALL")
Reported-by: Ross Lagerwall <ross.lagerwall@xxxxxxxxxx>
Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
---
CC: Jan Beulich <jbeulich@xxxxxxxx>
CC: Roger Pau Monné <roger.pau@xxxxxxxxxx>
CC: Teddy Astie <teddy.astie@xxxxxxxxxx>
CC: Ross Lagerwall <ross.lagerwall@xxxxxxxxxx>
---
 xen/arch/x86/x86_64/asm-offsets.c | 1 +
 xen/arch/x86/x86_64/entry.S       | 4 +++-
 2 files changed, 4 insertions(+), 1 deletion(-)

diff --git a/xen/arch/x86/x86_64/asm-offsets.c 
b/xen/arch/x86/x86_64/asm-offsets.c
index 9d4536402661..baf266ab8013 100644
--- a/xen/arch/x86/x86_64/asm-offsets.c
+++ b/xen/arch/x86/x86_64/asm-offsets.c
@@ -64,6 +64,7 @@ void __dummy__(void)
     DEFINE(sym, offsetof(struct cpu_user_regs, mem) -                   \
                 offsetof(struct cpu_user_regs, error_code) __VA_ARGS__)
 
+    OFFSET_EF(EFRAME_error_code,      error_code);
     OFFSET_EF(EFRAME_entry_vector,    entry_vector);
     OFFSET_EF(EFRAME_rip,             rip);
     OFFSET_EF(EFRAME_cs,              cs);
diff --git a/xen/arch/x86/x86_64/entry.S b/xen/arch/x86/x86_64/entry.S
index 22729b1f43b8..59953c9f525a 100644
--- a/xen/arch/x86/x86_64/entry.S
+++ b/xen/arch/x86/x86_64/entry.S
@@ -765,7 +765,9 @@ UNLIKELY_END(exit_cr3)
          * scf and ver_sel above eflags, as we can't use any GPRs,
          * and we're at a random place on the stack, not in a CPUFINFO block.
          */
-        SPEC_CTRL_COND_VERW     /* Req: %rsp=eframe                    Clob: 
efl */
+        SPEC_CTRL_COND_VERW     /* Req: %rsp=eframe                    Clob: 
efl */ \
+            scf=STK_REL(EFRAME_shadow_scf, EFRAME_error_code),                 
     \
+            sel=STK_REL(EFRAME_shadow_sel, EFRAME_error_code)
 
         add     $8, %rsp        /* Pop ev/ec off the stack */
         iretq

base-commit: 1cc1f4d7a0471f3bf9126ee7b1956db66ee28b58
-- 
2.39.5




 


Rackspace

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