Re: [PATCH v6 11/20] KVM: x86: Handle EGPR index and REX2-incompatible opcodes

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Incomplete NoRex2 enforcement on legacy instructions and architectural flaw in flag checking prevents group instruction support.
- [Medium] Missing NoRex2 flag on legacy SIMD instructions creates a future Guest-to-Host DoS (Kernel Panic) via out-of-bounds register read.
--

--- Patch [11]: [PATCH v6 11/20] KVM: x86: Handle EGPR index and REX2-incompatible opcodes ---
commit 60f040dc7da6f4fa4728dd8cf6193faca78101de
Author: Chang S. Bae <chang.seok.bae@intel.com>

KVM: x86: Handle EGPR index and REX2-incompatible opcodes

Prepare the emulator for REX2 handling by introducing the NoRex2 opcode
flag and supporting extended register indices. For the latter, factor out
common logic for calculating register IDs.

Link: https://lore.kernel.org/1ebf3a23-5671-41c1-8daa-c83f2f105936@redhat.com

> diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
> index b566ab5c75156..12a822870d08e 100644
> --- a/arch/x86/kvm/emulate.c
> +++ b/arch/x86/kvm/emulate.c

[ ... ]

> @@ -1118,7 +1129,7 @@ static void decode_register_operand(struct x86_emulate_ctxt *ctxt,
>  	if (ctxt->d & ModRM)
>  		reg = ctxt->modrm_reg;
>  	else
> -		reg = (ctxt->b & 7) | (ctxt->rex_bits & REX_B ? 8 : 0);
> +		reg = (ctxt->b & 7) | rex_get_rxb(ctxt->rex_bits, REX_B);
>  
>  	__decode_register_operand(ctxt, op, reg);
>  }

[Severity: Medium]
Are legacy SSE and AVX instructions (e.g., MOVAPS at 0x0F 0x28 in
twobyte_table) missing the NoRex2 flag in the emulation tables?

If a guest executes a REX2-prefixed legacy SIMD instruction, the new
rex_get_rxb() logic would extract extended bits, producing a register
index between 16 and 31.

During operand decoding, this out-of-bounds index is passed along:

arch/x86/kvm/emulate.c:__decode_register_operand() {
    ...
    if (ctxt->d & (Avx|Sse)) {
        op->type = OP_XMM;
        op->bytes = 16;
        op->addr.xmm = reg;
        kvm_read_sse_reg(reg, &op->vec_val);
    ...
}

Will this pass the out-of-bounds index (16-31) to kvm_read_sse_reg()? 

The underlying accessor _kvm_read_sse_reg() only handles indices 0-15 and
hits a default BUG() case for anything higher, which would result in a host
kernel panic.

[ ... ]

> @@ -4994,6 +5005,13 @@ int x86_decode_insn(struct x86_emulate_ctxt *ctxt, void *insn, int insn_len, int
>  		opcode = opcode_table[ctxt->b];
>  	}
>  
> +	/*
> +	 * Instructions marked with NoRex2 ignore a legacy REX prefix, but #UD
> +	 * should be raised when prefixed with REX2.
> +	 */
> +	if (opcode.flags & NoRex2 && ctxt->rex_prefix == REX2_PREFIX)
> +		opcode.flags = Undefined;
> +
>  	if (opcode.flags & ModRM)
>  		ctxt->modrm = insn_fetch(u8, ctxt);
>  

[Severity: Medium]
Since this check is performed immediately after fetching the base opcode,
but before the while (ctxt->d & GroupMask) loop resolves nested group
instructions (such as call far in group 5), will this fail to enforce
the NoRex2 restriction on instructions nested within a group?

Should this check be evaluated against ctxt->d after the group decoding
loop instead?

Also, numerous legacy instructions (e.g., cpuid, hlt, cli, sti, pushf,
popf) appear to be missing the NoRex2 flag in the twobyte_table and
opcode_table, which might allow erroneous emulation instead of injecting
an undefined opcode exception.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260729191656.598771-1-chang.seok.bae@intel.com?part=11