Re: [PATCH v5 2/8] KVM: nSVM: Synthesize DecodeAssists EXITINFO for emulated intercepts

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:

New issues:
- [Medium] Unconditionally setting bit 63 for `MOV CRx` intercepts violates the AMD specification and causes L1 KVM to spuriously inject a #GP into 32-bit L2 guests.

Pre-existing issues:
- [High] Host kernel pointer leak (KASLR bypass) when emulating INVLPG with a register operand.
--

--- Patch [2]: [PATCH v5 2/8] KVM: nSVM: Synthesize DecodeAssists EXITINFO for emulated intercepts ---
commit 8b2926ca93c673e51f89c3e65becdb72f34b66f7
Author: Tina Zhang <[email protected]>

KVM: nSVM: Synthesize DecodeAssists EXITINFO for emulated intercepts

When the x86 emulator encounters an instruction intercepted by L1,
svm_check_intercept() synthesizes a nested VM-Exit without fresh hardware
DecodeAssist state.  Populate the architectural EXITINFO fields when
DecodeAssists is exposed to L1.

> diff --git a/arch/x86/kvm/svm/svm.c b/arch/x86/kvm/svm/svm.c
> index 9d607b98bd067..c7c1f1527c3cf 100644
> --- a/arch/x86/kvm/svm/svm.c
> +++ b/arch/x86/kvm/svm/svm.c
> @@ -4810,6 +4810,52 @@ static const struct __x86_intercept {

[ ... ]

> +	switch (info->intercept) {
> +	case x86_intercept_cr_read:
> +	case x86_intercept_cr_write:
> +		/* MOV CRx: bit 63 set, GPR number in bits 3:0. */
> +		exit_info_1 = BIT_ULL(63) | (info->modrm_rm & 0xf);

[Severity: Medium]
Does unconditionally setting bit 63 here violate the AMD specification for
32-bit L2 guests?

If an L2 guest running in 32-bit mode issues a MOV CRx instruction that gets
emulated by L0 (e.g. via KVM_FEP), L0 will synthesize a nested VM-Exit to L1
and unconditionally set bit 63 (CR_VALID) in EXITINFO1.

L1 KVM will see bit 63 set, assume it is a 64-bit operation, read the full
64-bit GPR (which may contain garbage in the upper 32 bits from 32-bit
execution), and pass it to kvm_set_cr4(). This would fail the reserved-bit
check, causing L1 to spuriously inject a #GP into the 32-bit L2 guest.

> +		break;
> +	case x86_intercept_clts:

[ ... ]

> +	case x86_intercept_intn:
> +		/* INTn: software interrupt number in bits 7:0. */
> +		exit_info_1 = info->src_val & 0xff;
> +		break;
> +	case x86_intercept_invlpg:
> +		/* INVLPG: linear address of the target page. */
> +		exit_info_1 = info->intercept_linear_addr;

[Severity: High]
This is a pre-existing issue, but does assigning info->intercept_linear_addr
here leak a host kernel pointer (KASLR bypass) to the L1 guest?

If an L2 guest places an INVLPG instruction with a register operand
(e.g. INVLPG %eax) across an MMIO page boundary, KVM is forced to emulate it.

decode_modrm() correctly identifies it as a register operand and stores a host
kernel pointer in addr.reg.

Later, get_intercept_linear_addr() blindly reads addr.mem.ea (which aliases
addr.reg), extracting the host kernel pointer:

arch/x86/kvm/emulate.c:get_intercept_linear_addr() {
    ...
    la = seg_base(ctxt, ctxt->src.addr.mem.seg) + ctxt->src.addr.mem.ea;
    ...
}

By writing info->intercept_linear_addr to exit_info_1 here, the host KASLR
offset is exposed to the L1 guest in the synthesized VM-Exit.

> +		break;
> +	case x86_intercept_invlpga:
> +		/* INVLPGA: the address remains available in guest rAX. */
> +		exit_info_1 = 0;
> +		break;
> +	default:
> +		return;
> +	}
> +
> +	vmcb->control.exit_info_1 = exit_info_1;
> +	vmcb->control.exit_info_2 = 0;
> +}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=2