LoongArch: KVM: EIOINTC: fix INT_ENCODE ipnum out-of-bounds access

arch/loongarch/kvm/intc/eiointc.c static void eiointc_set_sw_coreisr(struct loongarch_eiointc *s) · line 24

[Severity: High]
This is a pre-existing issue, but does eiointc_set_sw_coreisr() fail to decode cpuid in 1-hot mode when EIOINTC_ENABLE_CPU_ENCODE is disabled?

It looks like it fetches cpuid directly from s->coremap[irq] without decoding it. In the default 1-hot mode, cpuid represents a bitmask, but the subsequent call to kvm_get_vcpu_by_cpuid() expects an exact ID. Could this result in looking up the wrong vCPU during state restoration, causing lost interrupts?

[Severity: Medium]
This is a pre-existing issue, but is the lockless access to map->phys_map[cpuid] safe in kvm_get_vcpu_by_cpuid() when called here?

When kvm_get_vcpu_by_cpuid() reads map->phys_map[cpuid].enabled and vcpu, it doesn't use memory barriers like smp_load_acquire(). Concurrently, kvm_set_cpuid() updates these fields using a lock but without smp_store_release(). Could this compiler or CPU reordering cause kvm_get_vcpu_by_cpuid() to transiently return NULL?

[ ... ]

arch/loongarch/kvm/intc/eiointc.c static void eiointc_update_irq(struct loongarch_eiointc *s, int irq, int level) · line 51

[Severity: Critical]
This is a pre-existing issue, but is the irq index validated before being used for array accesses here?

The KVM MSI routing mechanism allows guests or host userspace to inject MSIs with an arbitrary 32-bit msi.data payload. In pch_msi_set_irq():

arch/loongarch/kvm/intc/pch_pic.c:pch_msi_set_irq() {
    ...
    eiointc_set_irq(kvm->arch.eiointc, e->msi.data, level);
    ...
}

The user-supplied e->msi.data is passed unvalidated to eiointc_set_irq() as the irq parameter. This unvalidated irq is then used as an index for bitwise operations like __set_bit(irq, isr) in eiointc_set_irq() and array accesses like s->sw_coremap[irq] in eiointc_update_irq(). Can this allow unprivileged guest code to perform arbitrary out-of-bounds memory writes on the host kernel heap?

[Severity: High]
This isn't a bug introduced by this patch, but does eiointc_update_irq() operate on the correct CPU when clearing a pending interrupt?

When a guest writes to EIOINTC_COREISR_START to clear a pending interrupt:

arch/loongarch/kvm/intc/eiointc.c:loongarch_eiointc_write() {
    ...
    eiointc_update_irq(s, irq + index * 64, 0);
    ...
}

The write handler issues the clear request without explicitly passing the original vCPU. Then eiointc_update_irq() dynamically fetches the target CPU from s->sw_coremap[irq]. If the guest has re-routed the IRQ while it was pending, sw_coremap will point to the new CPU. Will this clear the fast-path sw_coreisr state on the new CPU, leaving the old CPU with a permanently asserted parent IRQ and an unrecoverable interrupt storm?

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

arch/loongarch/kvm/intc/eiointc.c line 12

[Severity: Critical]
This is a pre-existing issue, but does this bit shift result in undefined behavior if an out-of-bounds irq is provided?

If a malicious guest or userspace provides an unvalidated MSI data payload, it is passed as the irq parameter through the following call chain:

kvm_send_userspace_msi() -> kvm_set_msi() -> pch_msi_set_irq()

pch_msi_set_irq() {
    ...
    eiointc_set_irq(kvm->arch.eiointc, e->msi.data, level);
}

The unvalidated irq is used in the bit shift above, and also leads to out-of-bounds array reads in eiointc_update_irq():

    cpu = s->sw_coremap[irq];

Could this also allow arbitrary host memory corruption when setting bits in eiointc_set_irq() since isr is bounded to EIOINTC_IRQS?

    level ? __set_bit(irq, isr) : __clear_bit(irq, isr);