Re: [PATCH] KVM: x86: Add IDT limit check in __emulate_int_real()

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] IDT limit check is bypassed for software interrupt vectors >= 128 due to sign-extension, leaving an out-of-bounds memory read and undefined behavior unaddressed.
--

commit 1d15914e95742042e4c5113eceda0a57155cbdd4
Author: Kayra Cizmeci <kayracizmeci@gmail.com>

KVM: x86: Add IDT limit check in __emulate_int_real()

During real mode interrupt emulation, KVM currently pushes data to the
stack before checking IDT limits. This commit fetches the IDT early and
injects a #GP if the limit is exceeded.

> diff --git a/arch/x86/kvm/emulate.c b/arch/x86/kvm/emulate.c
> index c8e292e9a24df..ca9685a778c56 100644
> --- a/arch/x86/kvm/emulate.c
> +++ b/arch/x86/kvm/emulate.c
> @@ -2009,7 +2009,11 @@ static int __emulate_int_real(struct x86_emulate_ctxt *ctxt, int irq)
>  	gva_t eip_addr;
>  	u16 cs, eip;
>  
> -	/* TODO: Add limit checks */
> +	ops->get_idt(ctxt, &dt);
> +
> +	if (dt.size < (irq << 2) + 3)
> +		return emulate_gp(ctxt, 0);

[Severity: High]
This is a pre-existing issue, but does this check protect against software
interrupt vectors >= 128?

The KVM instruction decoder treats the immediate operand of the INT n
instruction (opcode 0xcd) as a sign-extended byte (SrcImmByte in the opcode
table). This causes the decoder to sign-extend vectors like INT 0x80 to a
negative number.

When a negative vector is passed to __emulate_int_real() as the irq
parameter, (irq << 2) results in undefined behavior due to left-shifting a
negative integer.

Furthermore, dt.size is an unsigned short, which promotes to a signed int
during the comparison. If irq is negative, the right-hand side of the
comparison is negative, causing the dt.size check to evaluate to false and
silently bypass the intended protection.

This would then allow the emulator to calculate a negative offset for
dt.address + (irq << 2) and read out-of-bounds memory.

Could the decoder be updated to use SrcImmUByte for opcode 0xcd, or could
the irq parameter be handled as an unsigned value to prevent this bypass?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260608123503.23298-1-kayracizmeci@gmail.com?part=1