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

[Teddy Astie <teddy.astie@xxxxxxxxxx>]

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@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
Reviewed-by: Teddy Astie <teddy.astie@xxxxxxxxxx>

Attachment: OpenPGP_0x660FA9D102CBCFD0.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature.asc
Description: OpenPGP digital signature