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

[sashiko-bot@kernel.org]

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

New issues:
- [Critical] Missing `NoRex2` flags on vector instructions combined with extended register indices causes out-of-bounds access and host kernel panics (`BUG()`).
- [High] The `NoRex2` mechanism misses numerous REX2-incompatible legacy opcodes and is structurally flawed for instructions within groups, allowing guests to bypass `#UD` injection.

Pre-existing issues:
- [High] KVM calculates an incorrect effective address when a memory operand uses an SIB byte with index=4 (100b) and the REX.X (or REX2.X4) bit is set.
--

--- Patch [11]: [PATCH v5 11/20] KVM: x86: Handle EGPR index and REX2-incompatible opcodes ---
commit 01ac33b49b5ac2e53b61a7aa1ed3d313429a8038
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: Critical]
Can this extended register index cause a host kernel panic?

With the newly introduced rex_get_rxb(), the register index can now reach 31
if extended bits (e.g. from a REX2 payload) are set.

For SSE/AVX vector instructions, which lack the NoRex2 flag, this index
is passed entirely unmasked to kvm_read_sse_reg() in
__decode_register_operand().

If an extended index > 15 reaches _kvm_read_sse_reg(), it triggers the
default BUG() path:

arch/x86/kvm/fpu.h:_kvm_read_sse_reg() {
    ...
    switch (reg) {
    case 0: *data = sse[0]; break;
    ...
    case 15: *data = sse[15]; break;
    default: BUG();
    }
}

Would an unprivileged guest be able to use a REX2 prefix on a vector
instruction to intentionally panic the host?

[ ... ]

> @@ -1137,9 +1148,9 @@ static int decode_modrm(struct x86_emulate_ctxt *ctxt,
>  	int rc = X86EMUL_CONTINUE;
>  	ulong modrm_ea = 0;
>  
> -	ctxt->modrm_reg = (ctxt->rex_bits & REX_R ? 8 : 0);
> -	index_reg = (ctxt->rex_bits & REX_X ? 8 : 0);
> -	base_reg = (ctxt->rex_bits & REX_B ? 8 : 0);
> +	ctxt->modrm_reg = rex_get_rxb(ctxt->rex_bits, REX_R);
> +	index_reg       = rex_get_rxb(ctxt->rex_bits, REX_X);
> +	base_reg        = rex_get_rxb(ctxt->rex_bits, REX_B);

[Severity: High]
This is a pre-existing issue, but does this logic calculate an incorrect
effective address when the SIB index is 4 and REX.X (or REX2.X4) is set?

According to the Intel SDM (Vol 2A), when the SIB index field is 100b
(indicating no index register), the REX.X bit must be ignored.

However, KVM initializes index_reg with the REX.X bit (value 8, or up to 24
with REX2). When OR'd with the SIB index of 4 later in decode_modrm(),
index_reg becomes 12 (or 28).

KVM then checks:
    if (index_reg != 4)
        modrm_ea += reg_read(ctxt, index_reg) << scale;

Since 12 != 4, wouldn't KVM mistakenly read r12 and add it to the effective
address, causing it to read or write incorrect MMIO devices or memory?

[ ... ]

> @@ -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;

[Severity: High]
Does this check fail to inject #UD for numerous REX2-incompatible opcodes?

This check evaluates the NoRex2 flag immediately after reading the first opcode
byte, before the GroupMask evaluation loop.

If the opcode is a group descriptor (e.g. group5 in opcode_table), it does
not have the NoRex2 flag set. The group is resolved later, bypassing this #UD
check entirely. As a result, REX2-incompatible instructions inside groups like
far calls and far jumps cannot be properly trapped.

Additionally, ModR/M-less implicit instructions like hlt, cli, and sti appear
to be missing the NoRex2 flag entirely in opcode_table.

Will this structural flaw allow guests to bypass #UD injection for these
instructions?

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