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

Andrew Cooper posted 1 patch 1 week, 1 day ago
Patches applied successfully (tree, apply log)
git fetch https://gitlab.com/xen-project/patchew/xen tags/patchew/20260715104854.1813130-1-andrew.cooper3@citrix.com
xen/arch/x86/x86_64/asm-offsets.c | 1 +
xen/arch/x86/x86_64/entry.S       | 4 +++-
2 files changed, 4 insertions(+), 1 deletion(-)
[PATCH] x86/entry: Fix VERW offsets in restore_all_xen()
Posted by Andrew Cooper 1 week, 1 day ago
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@citrix.com>
Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
---
CC: Jan Beulich <jbeulich@suse.com>
CC: Roger Pau Monné <roger.pau@citrix.com>
CC: Teddy Astie <teddy.astie@vates.tech>
CC: Ross Lagerwall <ross.lagerwall@citrix.com>
---
 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


Re: [PATCH] x86/entry: Fix VERW offsets in restore_all_xen()
Posted by Andrew Cooper 1 week, 1 day ago
On 15/07/2026 11:48 am, Andrew Cooper wrote:
> Fixes: 8af337dfb8e2 ("x86/entry: Use POP_GPRS and remove RESTORE_ALL")
> Reported-by: Ross Lagerwall <ross.lagerwall@citrix.com>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> CC: Jan Beulich <jbeulich@suse.com>
> CC: Roger Pau Monné <roger.pau@citrix.com>
> CC: Teddy Astie <teddy.astie@vates.tech>
> CC: Ross Lagerwall <ross.lagerwall@citrix.com>

Ross has given me a T-by tag in private, but is having email troubles
right now.

~Andrew

Re: [PATCH] x86/entry: Fix VERW offsets in restore_all_xen()
Posted by Jan Beulich 1 week, 1 day ago
On 15.07.2026 12:48, Andrew Cooper wrote:
> 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@citrix.com>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>
Re: [PATCH] x86/entry: Fix VERW offsets in restore_all_xen()
Posted by Teddy Astie 1 week, 1 day ago
Le 15/07/2026 à 12:53, Andrew Cooper a écrit :
> 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@citrix.com>
> Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com>
> ---
> CC: Jan Beulich <jbeulich@suse.com>
> CC: Roger Pau Monné <roger.pau@citrix.com>
> CC: Teddy Astie <teddy.astie@vates.tech>
> CC: Ross Lagerwall <ross.lagerwall@citrix.com>
> ---
>   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

Reviewed-by: Teddy Astie <teddy.astie@vates.tech>