Re: [PATCH v4 4/4] LoongArch: KVM: Use internal API to deliver interrupt in kernel mode

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [Medium] Undocumented Security Fix / Out-of-Bounds Access
- [Medium] Undocumented API Contract Change for KVM_INTERRUPT
--

--- Patch [4]: [PATCH v4 4/4] LoongArch: KVM: Use internal API to deliver interrupt in kernel mode ---
commit b41f003908862410927912c7ede144c461f9fa1e
Author: Bibo Mao <maobibo@loongson.cn>

LoongArch: KVM: Use internal API to deliver interrupt in kernel mode

This commit updates the KVM LoongArch code to use internal APIs
kvm_queue_irq() and kvm_dequeue_irq() for kernel-mode interrupt delivery,
reserving kvm_vcpu_ioctl_interrupt() for user-mode. It also moves IPI
injection and ack operations inside lock protection to avoid contention.

> diff --git a/arch/loongarch/kvm/intc/eiointc.c b/arch/loongarch/kvm/intc/eiointc.c
> index fd089d54da103..e04aadbd68db9 100644
> --- a/arch/loongarch/kvm/intc/eiointc.c
> +++ b/arch/loongarch/kvm/intc/eiointc.c
> @@ -17,7 +17,8 @@ static void eiointc_set_sw_coreisr(struct loongarch_eiointc *s)
>  		if (!(s->status & BIT(EIOINTC_ENABLE_INT_ENCODE))) {
>  			ipnum = count_trailing_zeros(ipnum);
>  			ipnum = ipnum < 4 ? ipnum : 0;
> -		}
> +		} else if (ipnum >= LOONGSON_IP_NUM)
> +			ipnum = 0;

[Severity: Medium]
This isn't a bug introduced by this patch, but the commit message doesn't
mention adding these bounds checks. 

Since ipnum is used as an array index into the sw_coreisr array shortly after
this in eiointc_set_sw_coreisr(), does this fix a guest-triggerable
out-of-bounds access?

If so, should this security fix be documented in the commit message or perhaps
split into a separate patch?

[ ... ]
> @@ -36,13 +37,13 @@ static void eiointc_update_irq(struct loongarch_eiointc *s, int irq, int level)
>  {
>  	int ipnum, cpu, found;
>  	struct kvm_vcpu *vcpu;
> -	struct kvm_interrupt vcpu_irq;
>  
>  	ipnum = (s->ipmap >> (irq / 32 * 8)) & 0xff;
>  	if (!(s->status & BIT(EIOINTC_ENABLE_INT_ENCODE))) {
>  		ipnum = count_trailing_zeros(ipnum);
>  		ipnum = ipnum < 4 ? ipnum : 0;
> -	}
> +	} else if (ipnum >= LOONGSON_IP_NUM)
> +		ipnum = 0;

[Severity: Medium]
Similarly, this bounds check in eiointc_update_irq() appears to prevent
out-of-bounds accesses but is undocumented in the commit message.

[ ... ]
> diff --git a/arch/loongarch/kvm/vcpu.c b/arch/loongarch/kvm/vcpu.c
> index 1a0cf6c81df6a..f81dadc62c986 100644
> --- a/arch/loongarch/kvm/vcpu.c
> +++ b/arch/loongarch/kvm/vcpu.c
> @@ -1465,6 +1465,9 @@ int kvm_vcpu_ioctl_interrupt(struct kvm_vcpu *vcpu, struct kvm_interrupt *irq)
>  	if (vector >= EXCCODE_INT_NUM)
>  		return -EINVAL;
>  
> +	if (kvm_arch_irqchip_in_kernel(vcpu->kvm))
> +		return -EINVAL;
> +

[Severity: Medium]
Does this change break backwards compatibility for any userspace applications
that might be calling the KVM_INTERRUPT ioctl while the in-kernel irqchip
is enabled?

While it makes architectural sense to prevent userspace from concurrently
injecting raw interrupts in this state, the commit message does not explicitly
mention this change to the user-facing API contract. Could the commit message 
be updated to clearly document this new restriction?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260709082109.1361767-1-maobibo@loongson.cn?part=4