Re: [PATCH 2/3] KVM: PPC: Book3S HV: XICS: Update irq_type for cleanup to work

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] xive: host kernel panic in kvmppc_xive_cleanup_vcpu() due to uninitialized eoi_mmio
--

commit 2d2dc3956268ca839bb2cf6d3521ccdcf2a1831b
Author: Gautam Menghani <gautam@linux.ibm.com>

KVM: PPC: Book3S HV: XICS: Update irq_type for cleanup to work

This commit updates the assignment of vcpu->arch.irq_type to occur before
vcpu configuration starts. This is intended to ensure that
kvmppc_xive_cleanup_vcpu() can properly execute if early setup failures are
encountered during kvmppc_xive_connect_vcpu().

> diff --git a/arch/powerpc/kvm/book3s_xive.c b/arch/powerpc/kvm/book3s_xive.c
> index 1d67237783b71..67219044fb8c7 100644
> --- a/arch/powerpc/kvm/book3s_xive.c
> +++ b/arch/powerpc/kvm/book3s_xive.c
> @@ -1937,6 +1937,7 @@ int kvmppc_xive_connect_vcpu(struct kvm_device *dev,
>  	xc->vp_id = vp_id;
>  	xc->mfrr = 0xff;
>  	xc->valid = true;
> +	vcpu->arch.irq_type = KVMPPC_IRQ_XICS;

[Severity: Critical]
Could this cause a regression where an early failure leads to a host
kernel panic?

If xive_native_get_vp_info() or xive_native_alloc_irq() fails shortly after
this assignment, the code jumps to the bail label and calls
kvmppc_xive_cleanup_vcpu(). 

Because irq_type is now KVMPPC_IRQ_XICS, kvmppc_xive_cleanup_vcpu() no
longer bails out early, and unconditionally attempts to mask the VP IPI:

kvmppc_xive_cleanup_vcpu()
    /* Mask the VP IPI */
    xive_vm_esb_load(&xc->vp_ipi_data, XIVE_ESB_SET_PQ_01);

Since xive_native_populate_irq_data() has not been successfully executed yet,
xc->vp_ipi_data.eoi_mmio is still uninitialized. When xive_vm_esb_load()
is called:

xive_vm_esb_load()
    val = __raw_readq(__x_eoi_page(xd) + offset);

Would this dereference the uninitialized eoi_mmio pointer and cause a
fatal page fault?

Could an unprivileged host userspace process intentionally trigger this
early failure path (e.g., by exhausting IRQ resources) during VM creation,
resulting in a host denial of service?

>  
>  	r = xive_native_get_vp_info(xc->vp_id, &xc->vp_cam, &xc->vp_chip_id);
>  	if (r)
[ ... ]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261001100208.1271628-1-gautam@linux.ibm.com?part=2