-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA256
Xen Security Advisory CVE-2023-46842 / XSA-454
version 2
x86 HVM hypercalls may trigger Xen bug check
UPDATES IN VERSION 2
====================
Avoid new Misra violation in 1st staging patch.
Public release.
ISSUE DESCRIPTION
=================
Unlike 32-bit PV guests, HVM guests may switch freely between 64-bit and
other modes. This in particular means that they may set registers used
to pass 32-bit-mode hypercall arguments to values outside of the range
32-bit code would be able to set them to.
When processing of hypercalls takes a considerable amount of time,
the hypervisor may choose to invoke a hypercall continuation. Doing so
involves putting (perhaps updated) hypercall arguments in respective
registers. For guests not running in 64-bit mode this further involves
a certain amount of translation of the values.
Unfortunately internal sanity checking of these translated values
assumes high halves of registers to always be clear when invoking a
hypercall. When this is found not to be the case, it triggers a
consistency check in the hypervisor and causes a crash.
IMPACT
======
A HVM or PVH guest can cause a hypervisor crash, causing a Denial of
Service (DoS) of the entire host.
VULNERABLE SYSTEMS
==================
All Xen versions from at least 3.2 onwards are vulnerable. Earlier
versions have not been inspected.
Only x86 systems are vulnerable. Arm systems are not vulnerable.
Only HVM or PVH guests can leverage the vulnerability. PV guests cannot
leverage the vulnerability.
MITIGATION
==========
Not using HVM / PVH guests will avoid the vulnerability.
CREDITS
=======
This issue was discovered by Manuel Andreas of Technical University of
Munich.
RESOLUTION
==========
Applying either of the attached patches from the appropriate set resolves
this issue.
Note that patches for released versions are generally prepared to
apply to the stable branches, and may not apply cleanly to the most
recent release tarball. Downstreams are encouraged to update to the
tip of the stable branch before applying these patches.
xsa454-?.patch xen-unstable
xsa454-4.18-?.patch Xen 4.18.x
xsa454-4.17-?.patch Xen 4.17.x
xsa454-4.16-?.patch Xen 4.16.x - Xen 4.15.x
$ sha256sum xsa454*
2df9af16605b634d3585a30f673b4cf9e327889cfd8714a697de215c3f809fb5 xsa454-1.patch
f2ed0468350f2c2e0285a546ab5c722e928add5425b05bff663c632ada09ee3b xsa454-2.patch
4106f323251e262d30319c61de7c876f2b18edfcce38cc70501fb3c22677ff0a xsa454-4.16-1.patch
962ea7d8f3e378ec775619e44525f66768369423b56113420763651dbbf6bc1e xsa454-4.16-2.patch
95b299237d13ae27f643d804eb40b600b9b8ef056953686d4f770f03c46c42c8 xsa454-4.17-1.patch
7af290595cbea3153e49344827095c874e6a8d208d8c843e62ee0787b0d7d46d xsa454-4.17-2.patch
999006e7917c996741dfc332d28e7b2ca8376f8e9d5b38161cbd5988528d0238 xsa454-4.18-1.patch
f2ed0468350f2c2e0285a546ab5c722e928add5425b05bff663c632ada09ee3b xsa454-4.18-2.patch
$
DEPLOYMENT DURING EMBARGO
=========================
Deployment of the patches and/or mitigations described above (or
others which are substantially similar) is permitted during the
embargo, even on public-facing systems with untrusted guest users and
administrators.
But: Distribution of updated software is prohibited (except to other
members of the predisclosure list).
Predisclosure list members who wish to deploy significantly different
patches and/or mitigations, please contact the Xen Project Security
Team.
(Note: this during-embargo deployment notice is retained in
post-embargo publicly released Xen Project advisories, even though it
is then no longer applicable. This is to enable the community to have
oversight of the Xen Project Security Team's decisionmaking.)
For more information about permissible uses of embargoed information,
consult the Xen Project community's agreed Security Policy:
http://www.xenproject.org/security-policy.html
-----BEGIN PGP SIGNATURE-----
iQFABAEBCAAqFiEEI+MiLBRfRHX6gGCng/4UyVfoK9kFAmYVK4QMHHBncEB4ZW4u
b3JnAAoJEIP+FMlX6CvZGckH/3BlZCckKISpUFMM/633xyAdJ8ZMVwZDhS2/eC+n
SJA4VuqAgw6dqqvAA5ga7jzBiCxe78S1BVXAZjOctmfVHRTOoyKg2hcEcKAit8uf
Pbxm/XHqgQRb6FTAlZROqX0rxq+7kSftm0teQWvMfauwVia59Shhye67dmdk9tCP
G8BTDFVEAspFYopQOiTmFQbxIkLLC6rg0UljQfxStPMw3MyX8pO5Lzl3+POlM1xV
XBynHxVmpdXNe1rFYcRKIsQWbbgYiEMXjQmOkax2mTfMHDhMZjkxvpLZa2jMfzkP
wTdqwWqO+z2eGZPWVL95uwZ49Q6Pzhnd6MXkn0wfHtDzy24=
=oUIS
-----END PGP SIGNATURE-----
From: Jan Beulich <jbeulich@suse.com>
Subject: x86/HVM: clear upper halves of GPRs upon entry from 32-bit code
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: b8a7efe8528a ("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>
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -2603,7 +2603,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);
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -4033,6 +4033,7 @@ static void undo_nmis_unblocked_by_iret(
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;
@@ -4041,7 +4042,10 @@ void asmlinkage vmx_vmexit_handler(struc
__vmread(GUEST_RSP, ®s->rsp);
__vmread(GUEST_RFLAGS, ®s->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) )
{
--- a/xen/arch/x86/include/asm/hvm/hvm.h
+++ b/xen/arch/x86/include/asm/hvm/hvm.h
@@ -569,8 +569,24 @@ static inline unsigned int hvm_get_insn_
? 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->rbp;
+ regs->rbx = (uint32_t)regs->rbx;
+ regs->rax = (uint32_t)regs->rax;
+ regs->rcx = (uint32_t)regs->rcx;
+ regs->rdx = (uint32_t)regs->rdx;
+ regs->rsi = (uint32_t)regs->rsi;
+ regs->rdi = (uint32_t)regs->rdi;
+ regs->rip = (uint32_t)regs->rip;
+ regs->rflags = (uint32_t)regs->rflags;
+ regs->rsp = (uint32_t)regs->rsp;
+ }
+
#ifndef NDEBUG
regs->error_code = 0xbeef;
regs->entry_vector = 0xbeef;
From: Bjoern Doebel <doebel@amazon.de>
Subject: hypercall_xlat_continuation: Replace BUG_ON with domain_crash
Instead of crashing the host in case of unexpected hypercall parameters,
resort to only crashing the calling domain.
This is part of XSA-454 / CVE-2023-46842.
Fixes: b8a7efe8528a ("Enable compatibility mode operation for HYPERVISOR_memory_op")
Reported-by: Manuel Andreas <manuel.andreas@tum.de>
Signed-off-by: Bjoern Doebel <doebel@amazon.de>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
--- a/xen/arch/x86/hypercall.c
+++ b/xen/arch/x86/hypercall.c
@@ -140,8 +140,10 @@ int hypercall_xlat_continuation(unsigned
cval = va_arg(args, unsigned int);
if ( cval == nval )
mask &= ~1U;
- else
- BUG_ON(nval == (unsigned int)nval);
+ else if ( nval == (unsigned int)nval )
+ domain_crash(current->domain,
+ "multicall (op %lu) bogus continuation arg%u (%#lx)\n",
+ mcs->call.op, i, nval);
}
else if ( id && *id == i )
{
@@ -153,8 +155,10 @@ int hypercall_xlat_continuation(unsigned
mcs->call.args[i] = cval;
++rc;
}
- else
- BUG_ON(mcs->call.args[i] != (unsigned int)mcs->call.args[i]);
+ else if ( mcs->call.args[i] != (unsigned int)mcs->call.args[i] )
+ domain_crash(current->domain,
+ "multicall (op %lu) bad continuation arg%u (%#lx)\n",
+ mcs->call.op, i, mcs->call.args[i]);
}
}
else
@@ -180,8 +184,10 @@ int hypercall_xlat_continuation(unsigned
cval = va_arg(args, unsigned int);
if ( cval == nval )
mask &= ~1U;
- else
- BUG_ON(nval == (unsigned int)nval);
+ else if ( nval == (unsigned int)nval )
+ domain_crash(current->domain,
+ "hypercall (op %u) bogus continuation arg%u (%#lx)\n",
+ regs->eax, i, nval);
}
else if ( id && *id == i )
{
@@ -193,8 +199,10 @@ int hypercall_xlat_continuation(unsigned
*reg = cval;
++rc;
}
- else
- BUG_ON(*reg != (unsigned int)*reg);
+ else if ( *reg != (unsigned int)*reg )
+ domain_crash(current->domain,
+ "hypercall (op %u) bad continuation arg%u (%#lx)\n",
+ regs->eax, i, *reg);
}
}
From: Jan Beulich <jbeulich@suse.com>
Subject: x86/HVM: clear upper halves of GPRs upon entry from 32-bit code
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: b8a7efe8528a ("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>
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -2593,7 +2593,8 @@ void svm_vmexit_handler(struct cpu_user_
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);
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -3939,6 +3939,7 @@ static int vmx_handle_apic_write(void)
void 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, mode;
struct vcpu *v = current;
struct domain *currd = v->domain;
@@ -3947,7 +3948,10 @@ void vmx_vmexit_handler(struct cpu_user_
__vmread(GUEST_RSP, ®s->rsp);
__vmread(GUEST_RFLAGS, ®s->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) )
{
--- a/xen/include/asm-x86/hvm/hvm.h
+++ b/xen/include/asm-x86/hvm/hvm.h
@@ -579,8 +579,24 @@ static inline unsigned int hvm_get_insn_
? 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 = regs->ebp;
+ regs->rbx = regs->ebx;
+ regs->rax = regs->eax;
+ regs->rcx = regs->ecx;
+ regs->rdx = regs->edx;
+ regs->rsi = regs->esi;
+ regs->rdi = regs->edi;
+ regs->rip = regs->eip;
+ regs->rflags = regs->eflags;
+ regs->rsp = regs->esp;
+ }
+
#ifndef NDEBUG
regs->error_code = 0xbeef;
regs->entry_vector = 0xbeef;
From: Bjoern Doebel <doebel@amazon.de>
Subject: hypercall_xlat_continuation: Replace BUG_ON with domain_crash
Instead of crashing the host in case of unexpected hypercall parameters,
resort to only crashing the calling domain.
This is part of XSA-454 / CVE-2023-46842.
Fixes: b8a7efe8528a ("Enable compatibility mode operation for HYPERVISOR_memory_op")
Reported-by: Manuel Andreas <manuel.andreas@tum.de>
Signed-off-by: Bjoern Doebel <doebel@amazon.de>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
--- a/xen/arch/x86/hypercall.c
+++ b/xen/arch/x86/hypercall.c
@@ -211,8 +211,13 @@ int hypercall_xlat_continuation(unsigned
cval = va_arg(args, unsigned int);
if ( cval == nval )
mask &= ~1U;
- else
- BUG_ON(nval == (unsigned int)nval);
+ else if ( nval == (unsigned int)nval )
+ {
+ printk(XENLOG_G_ERR
+ "multicall (op %lu) bogus continuation arg%u (%#lx)\n",
+ mcs->call.op, i, nval);
+ domain_crash(current->domain);
+ }
}
else if ( id && *id == i )
{
@@ -224,8 +229,13 @@ int hypercall_xlat_continuation(unsigned
mcs->call.args[i] = cval;
++rc;
}
- else
- BUG_ON(mcs->call.args[i] != (unsigned int)mcs->call.args[i]);
+ else if ( mcs->call.args[i] != (unsigned int)mcs->call.args[i] )
+ {
+ printk(XENLOG_G_ERR
+ "multicall (op %lu) bad continuation arg%u (%#lx)\n",
+ mcs->call.op, i, mcs->call.args[i]);
+ domain_crash(current->domain);
+ }
}
}
else
@@ -251,8 +261,13 @@ int hypercall_xlat_continuation(unsigned
cval = va_arg(args, unsigned int);
if ( cval == nval )
mask &= ~1U;
- else
- BUG_ON(nval == (unsigned int)nval);
+ else if ( nval == (unsigned int)nval )
+ {
+ printk(XENLOG_G_ERR
+ "hypercall (op %u) bogus continuation arg%u (%#lx)\n",
+ regs->eax, i, nval);
+ domain_crash(current->domain);
+ }
}
else if ( id && *id == i )
{
@@ -264,8 +279,13 @@ int hypercall_xlat_continuation(unsigned
*reg = cval;
++rc;
}
- else
- BUG_ON(*reg != (unsigned int)*reg);
+ else if ( *reg != (unsigned int)*reg )
+ {
+ printk(XENLOG_G_ERR
+ "hypercall (op %u) bad continuation arg%u (%#lx)\n",
+ regs->eax, i, *reg);
+ domain_crash(current->domain);
+ }
}
}
From: Jan Beulich <jbeulich@suse.com>
Subject: x86/HVM: clear upper halves of GPRs upon entry from 32-bit code
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: b8a7efe8528a ("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>
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -2626,7 +2626,8 @@ void svm_vmexit_handler(struct cpu_user_
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);
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -3981,6 +3981,7 @@ static void undo_nmis_unblocked_by_iret(
void 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;
@@ -3989,7 +3990,10 @@ void vmx_vmexit_handler(struct cpu_user_
__vmread(GUEST_RSP, ®s->rsp);
__vmread(GUEST_RFLAGS, ®s->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) )
{
--- a/xen/arch/x86/include/asm/hvm/hvm.h
+++ b/xen/arch/x86/include/asm/hvm/hvm.h
@@ -583,8 +583,24 @@ static inline unsigned int hvm_get_insn_
? 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 = regs->ebp;
+ regs->rbx = regs->ebx;
+ regs->rax = regs->eax;
+ regs->rcx = regs->ecx;
+ regs->rdx = regs->edx;
+ regs->rsi = regs->esi;
+ regs->rdi = regs->edi;
+ regs->rip = regs->eip;
+ regs->rflags = regs->eflags;
+ regs->rsp = regs->esp;
+ }
+
#ifndef NDEBUG
regs->error_code = 0xbeef;
regs->entry_vector = 0xbeef;
From: Bjoern Doebel <doebel@amazon.de>
Subject: hypercall_xlat_continuation: Replace BUG_ON with domain_crash
Instead of crashing the host in case of unexpected hypercall parameters,
resort to only crashing the calling domain.
This is part of XSA-454 / CVE-2023-46842.
Fixes: b8a7efe8528a ("Enable compatibility mode operation for HYPERVISOR_memory_op")
Reported-by: Manuel Andreas <manuel.andreas@tum.de>
Signed-off-by: Bjoern Doebel <doebel@amazon.de>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
--- a/xen/arch/x86/hypercall.c
+++ b/xen/arch/x86/hypercall.c
@@ -152,8 +152,13 @@ int hypercall_xlat_continuation(unsigned
cval = va_arg(args, unsigned int);
if ( cval == nval )
mask &= ~1U;
- else
- BUG_ON(nval == (unsigned int)nval);
+ else if ( nval == (unsigned int)nval )
+ {
+ printk(XENLOG_G_ERR
+ "multicall (op %lu) bogus continuation arg%u (%#lx)\n",
+ mcs->call.op, i, nval);
+ domain_crash(current->domain);
+ }
}
else if ( id && *id == i )
{
@@ -165,8 +170,13 @@ int hypercall_xlat_continuation(unsigned
mcs->call.args[i] = cval;
++rc;
}
- else
- BUG_ON(mcs->call.args[i] != (unsigned int)mcs->call.args[i]);
+ else if ( mcs->call.args[i] != (unsigned int)mcs->call.args[i] )
+ {
+ printk(XENLOG_G_ERR
+ "multicall (op %lu) bad continuation arg%u (%#lx)\n",
+ mcs->call.op, i, mcs->call.args[i]);
+ domain_crash(current->domain);
+ }
}
}
else
@@ -192,8 +202,13 @@ int hypercall_xlat_continuation(unsigned
cval = va_arg(args, unsigned int);
if ( cval == nval )
mask &= ~1U;
- else
- BUG_ON(nval == (unsigned int)nval);
+ else if ( nval == (unsigned int)nval )
+ {
+ printk(XENLOG_G_ERR
+ "hypercall (op %u) bogus continuation arg%u (%#lx)\n",
+ regs->eax, i, nval);
+ domain_crash(current->domain);
+ }
}
else if ( id && *id == i )
{
@@ -205,8 +220,13 @@ int hypercall_xlat_continuation(unsigned
*reg = cval;
++rc;
}
- else
- BUG_ON(*reg != (unsigned int)*reg);
+ else if ( *reg != (unsigned int)*reg )
+ {
+ printk(XENLOG_G_ERR
+ "hypercall (op %u) bad continuation arg%u (%#lx)\n",
+ regs->eax, i, *reg);
+ domain_crash(current->domain);
+ }
}
}
From: Jan Beulich <jbeulich@suse.com>
Subject: x86/HVM: clear upper halves of GPRs upon entry from 32-bit code
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: b8a7efe8528a ("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>
--- a/xen/arch/x86/hvm/svm/svm.c
+++ b/xen/arch/x86/hvm/svm/svm.c
@@ -2603,7 +2603,8 @@ void 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);
--- a/xen/arch/x86/hvm/vmx/vmx.c
+++ b/xen/arch/x86/hvm/vmx/vmx.c
@@ -4041,6 +4041,7 @@ static void undo_nmis_unblocked_by_iret(
void 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;
@@ -4049,7 +4050,10 @@ void vmx_vmexit_handler(struct cpu_user_
__vmread(GUEST_RSP, ®s->rsp);
__vmread(GUEST_RFLAGS, ®s->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) )
{
--- a/xen/arch/x86/include/asm/hvm/hvm.h
+++ b/xen/arch/x86/include/asm/hvm/hvm.h
@@ -579,8 +579,24 @@ static inline unsigned int hvm_get_insn_
? 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 = regs->ebp;
+ regs->rbx = regs->ebx;
+ regs->rax = regs->eax;
+ regs->rcx = regs->ecx;
+ regs->rdx = regs->edx;
+ regs->rsi = regs->esi;
+ regs->rdi = regs->edi;
+ regs->rip = regs->eip;
+ regs->rflags = regs->eflags;
+ regs->rsp = regs->esp;
+ }
+
#ifndef NDEBUG
regs->error_code = 0xbeef;
regs->entry_vector = 0xbeef;
From: Bjoern Doebel <doebel@amazon.de>
Subject: hypercall_xlat_continuation: Replace BUG_ON with domain_crash
Instead of crashing the host in case of unexpected hypercall parameters,
resort to only crashing the calling domain.
This is part of XSA-454 / CVE-2023-46842.
Fixes: b8a7efe8528a ("Enable compatibility mode operation for HYPERVISOR_memory_op")
Reported-by: Manuel Andreas <manuel.andreas@tum.de>
Signed-off-by: Bjoern Doebel <doebel@amazon.de>
Signed-off-by: Jan Beulich <jbeulich@suse.com>
Reviewed-by: Roger Pau Monné <roger.pau@citrix.com>
--- a/xen/arch/x86/hypercall.c
+++ b/xen/arch/x86/hypercall.c
@@ -140,8 +140,10 @@ int hypercall_xlat_continuation(unsigned
cval = va_arg(args, unsigned int);
if ( cval == nval )
mask &= ~1U;
- else
- BUG_ON(nval == (unsigned int)nval);
+ else if ( nval == (unsigned int)nval )
+ domain_crash(current->domain,
+ "multicall (op %lu) bogus continuation arg%u (%#lx)\n",
+ mcs->call.op, i, nval);
}
else if ( id && *id == i )
{
@@ -153,8 +155,10 @@ int hypercall_xlat_continuation(unsigned
mcs->call.args[i] = cval;
++rc;
}
- else
- BUG_ON(mcs->call.args[i] != (unsigned int)mcs->call.args[i]);
+ else if ( mcs->call.args[i] != (unsigned int)mcs->call.args[i] )
+ domain_crash(current->domain,
+ "multicall (op %lu) bad continuation arg%u (%#lx)\n",
+ mcs->call.op, i, mcs->call.args[i]);
}
}
else
@@ -180,8 +184,10 @@ int hypercall_xlat_continuation(unsigned
cval = va_arg(args, unsigned int);
if ( cval == nval )
mask &= ~1U;
- else
- BUG_ON(nval == (unsigned int)nval);
+ else if ( nval == (unsigned int)nval )
+ domain_crash(current->domain,
+ "hypercall (op %u) bogus continuation arg%u (%#lx)\n",
+ regs->eax, i, nval);
}
else if ( id && *id == i )
{
@@ -193,8 +199,10 @@ int hypercall_xlat_continuation(unsigned
*reg = cval;
++rc;
}
- else
- BUG_ON(*reg != (unsigned int)*reg);
+ else if ( *reg != (unsigned int)*reg )
+ domain_crash(current->domain,
+ "hypercall (op %u) bad continuation arg%u (%#lx)\n",
+ regs->eax, i, *reg);
}
}
© 2016 - 2024 Red Hat, Inc.