Re: [PATCH 2/6] LoongArch: KVM: Guard against NULL irqchip in irq injection

[Tao Cui <cui.tao@linux.dev>]

在 2026/9/28 12:14, Bibo Mao 写道:
> 
> 
> On 2026/9/27 下午3:52, Tao Cui wrote:
>> From: Tao Cui <cuitao@kylinos.cn>
>>
>> The irqfd injection path dispatches through the irq routing table
>> without the kvm_arch_irqchip_in_kernel() gate that protects
>> KVM_IRQ_LINE: kvm_set_pic_irq() and kvm_arch_set_irq_inatomic() call
>> pch_pic_set_irq(kvm->arch.pch_pic, ...) and pch_msi_set_irq() calls
>> eiointc_set_irq(kvm->arch.eiointc, ...) directly, so a routing entry
>> that outlives the corresponding in-kernel irqchip dereferences a NULL
>> (or, before the previous patch, a freed) device.
>>
>> Make pch_pic_set_irq(), eiointc_set_irq() and dmsintc_set_irq()
>> tolerate a NULL device.
>>
>> Fixes: 1928254c5ccb ("LoongArch: KVM: Add irqfd support")
>> Signed-off-by: Tao Cui <cuitao@kylinos.cn>
>> ---
>>   arch/loongarch/kvm/intc/dmsintc.c | 3 +++
>>   arch/loongarch/kvm/intc/eiointc.c | 5 ++++-
>>   arch/loongarch/kvm/intc/pch_pic.c | 3 +++
>>   3 files changed, 10 insertions(+), 1 deletion(-)
>>
>> diff --git a/arch/loongarch/kvm/intc/dmsintc.c b/arch/loongarch/kvm/intc/dmsintc.c
>> index 63072595c01b..e27f448bb54f 100644
>> --- a/arch/loongarch/kvm/intc/dmsintc.c
>> +++ b/arch/loongarch/kvm/intc/dmsintc.c
>> @@ -70,6 +70,9 @@ int dmsintc_set_irq(struct kvm *kvm, u64 addr, int data, int level)
>>       unsigned int irq, cpu;
>>       struct kvm_vcpu *vcpu;
>>   +    if (!kvm->arch.dmsintc)
>> +        return -EINVAL;
>> +
> There is kvm->arch.dmsintc checking in its caller function pch_msi_set_irq().
> 
>>       irq = (addr >> AVEC_IRQ_SHIFT) & AVEC_IRQ_MASK;
>>       cpu = (addr >> AVEC_CPU_SHIFT) & kvm->arch.dmsintc->cpu_mask;
>>       if (cpu >= KVM_MAX_VCPUS)
>> diff --git a/arch/loongarch/kvm/intc/eiointc.c b/arch/loongarch/kvm/intc/eiointc.c
>> index fe0a1918f26f..c68f9033638a 100644
>> --- a/arch/loongarch/kvm/intc/eiointc.c
>> +++ b/arch/loongarch/kvm/intc/eiointc.c
>> @@ -112,8 +112,11 @@ static inline void eiointc_update_sw_coremap(struct loongarch_eiointc *s,
>>   void eiointc_set_irq(struct loongarch_eiointc *s, int irq, int level)
>>   {
>>       unsigned long flags;
>> -    unsigned long *isr = (unsigned long *)s->isr;
>> +    unsigned long *isr;
>>   +    if (!s)
>> +        return;
> When is it possible that parameter s is NULL? There is kvm_arch_irqchip_in_kernel() checking in kvm_send_userspace_msi().
> 
>> +    isr = (unsigned long *)s->isr;
>>       spin_lock_irqsave(&s->lock, flags);
>>       level ? __set_bit(irq, isr) : __clear_bit(irq, isr);
>>       eiointc_update_irq(s, irq, level);
>> diff --git a/arch/loongarch/kvm/intc/pch_pic.c b/arch/loongarch/kvm/intc/pch_pic.c
>> index 62c09b5f3937..88666425d800 100644
>> --- a/arch/loongarch/kvm/intc/pch_pic.c
>> +++ b/arch/loongarch/kvm/intc/pch_pic.c
>> @@ -48,6 +48,9 @@ void pch_pic_set_irq(struct loongarch_pch_pic *s, int irq, int level)
>>   {
>>       u64 mask = BIT(irq);
>>   +    if (!s)
>> +        return;
>> +
> When is it possible that parameter s is NULL? I think that when in kernel irqchip is removed, there is no way to inject line IRQ in kernel side.
> 

Thanks for looking into this.

You're right. I rechecked the lifetime rules after your comments, and
my reasoning for this patch was flawed.

I had assumed an irqfd could remain active after the corresponding
in-kernel irqchip had been removed. In fact, that state is not
reachable. The irqchip devices are not destroyed during the lifetime
of a running VM, and during VM teardown irqfds are released before
the irqchip devices are destroyed. For DMSINTC, the rollback path is
already guarded by `pch_msi_set_irq()`.

So these NULL checks are not protecting a real execution path. I'll
drop this patch in the next revision.

Thanks,
Tao

> Regards
> Bibo Mao
>>       spin_lock(&s->lock);
>>       if (level)
>>           s->irr |= mask; /* set irr */
>>
>