Skip to content

Commit

Permalink
x86/HVM: clear upper halves of GPRs upon entry from 32-bit code
Browse files Browse the repository at this point in the history
Hypercalls in particular can be the subject of continuations, and logic
there checks updated state against incoming register values. If the
guest manufactured a suitable argument register with a non-zero upper
half before entering compatibility mode and issuing a hypercall from
there, checks in hypercall_xlat_continuation() might trip.

Since for HVM we want to also be sure to not hit a corner case in the
emulator, initiate the clipping right from the top of
{svm,vmx}_vmexit_handler(). Also rename the invoked function, as it no
longer does only invalidation of fields.

Note that architecturally the upper halves of registers are undefined
after a switch between compatibility and 64-bit mode (either direction).
Hence once having entered compatibility mode, the guest can't assume
the upper half of any register to retain its value.

This is part of XSA-454 / CVE-2023-46842.

Fixes: b8a7efe ("Enable compatibility mode operation for HYPERVISOR_memory_op")
Reported-by: Manuel Andreas <manuel.andreas@tum.de>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
  • Loading branch information
jbeulich authored and andyhhp committed Apr 9, 2024
1 parent 672b26b commit 6a98383
Show file tree
Hide file tree
Showing 3 changed files with 24 additions and 3 deletions.
3 changes: 2 additions & 1 deletion xen/arch/x86/hvm/svm/svm.c
Original file line number Diff line number Diff line change
Expand Up @@ -2546,7 +2546,8 @@ void asmlinkage svm_vmexit_handler(void)
regs->rsp = vmcb->rsp;
regs->rflags = vmcb->rflags;

hvm_invalidate_regs_fields(regs);
hvm_sanitize_regs_fields(
regs, !(vmcb_get_efer(vmcb) & EFER_LMA) || !(vmcb->cs.l));

if ( paging_mode_hap(v->domain) )
v->arch.hvm.guest_cr[3] = v->arch.hvm.hw_cr[3] = vmcb_get_cr3(vmcb);
Expand Down
6 changes: 5 additions & 1 deletion xen/arch/x86/hvm/vmx/vmx.c
Original file line number Diff line number Diff line change
Expand Up @@ -4033,6 +4033,7 @@ static void undo_nmis_unblocked_by_iret(void)
void asmlinkage vmx_vmexit_handler(struct cpu_user_regs *regs)
{
unsigned long exit_qualification, exit_reason, idtv_info, intr_info = 0;
unsigned long cs_ar_bytes = 0;
unsigned int vector = 0;
struct vcpu *v = current;
struct domain *currd = v->domain;
Expand All @@ -4041,7 +4042,10 @@ void asmlinkage vmx_vmexit_handler(struct cpu_user_regs *regs)
__vmread(GUEST_RSP, &regs->rsp);
__vmread(GUEST_RFLAGS, &regs->rflags);

hvm_invalidate_regs_fields(regs);
if ( hvm_long_mode_active(v) )
__vmread(GUEST_CS_AR_BYTES, &cs_ar_bytes);

hvm_sanitize_regs_fields(regs, !(cs_ar_bytes & X86_SEG_AR_CS_LM_ACTIVE));

if ( paging_mode_hap(v->domain) )
{
Expand Down
18 changes: 17 additions & 1 deletion xen/arch/x86/include/asm/hvm/hvm.h
Original file line number Diff line number Diff line change
Expand Up @@ -569,8 +569,24 @@ static inline unsigned int hvm_get_insn_bytes(struct vcpu *v, uint8_t *buf)
? alternative_call(hvm_funcs.get_insn_bytes, v, buf) : 0);
}

static inline void hvm_invalidate_regs_fields(struct cpu_user_regs *regs)
static inline void hvm_sanitize_regs_fields(struct cpu_user_regs *regs,
bool compat)
{
if ( compat )
{
/* Clear GPR upper halves, to counteract guests playing games. */
regs->rbp = (uint32_t)regs->ebp;
regs->rbx = (uint32_t)regs->ebx;
regs->rax = (uint32_t)regs->eax;
regs->rcx = (uint32_t)regs->ecx;
regs->rdx = (uint32_t)regs->edx;
regs->rsi = (uint32_t)regs->esi;
regs->rdi = (uint32_t)regs->edi;
regs->rip = (uint32_t)regs->eip;
regs->rflags = (uint32_t)regs->eflags;
regs->rsp = (uint32_t)regs->esp;
}

#ifndef NDEBUG
regs->error_code = 0xbeef;
regs->entry_vector = 0xbeef;
Expand Down

0 comments on commit 6a98383

Please sign in to comment.