Re: [PATCH v5 6/8] PCI: vmd: Convert bus and busn_start to an array

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Failing to call pci_free_irq_vectors() in error paths and driver removal leaks IRQ resources.
- [High] SRCU structures are destroyed before IRQ vectors are explicitly disabled, risking a Use-After-Free on spurious interrupts.
--

commit 5baf3e06c174f82b0781954bbca2834e43441922
Author: Szymon Durawa <szymon.durawa@linux.intel.com>

PCI: vmd: Convert bus and busn_start to an array

Convert bus and busn_start from scalar to an array to support multiple VMD
buses in the future. No functional changes.

> diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c
> index 9ff82a24e859c..2bfc16875d912 100644
> --- a/drivers/pci/controller/vmd.c
> +++ b/drivers/pci/controller/vmd.c

[ ... ]

> @@ -1114,9 +1122,9 @@ static void vmd_remove(struct pci_dev *dev)
>  {
>  	struct vmd_dev *vmd = pci_get_drvdata(dev);
>  
> -	pci_stop_root_bus(vmd->bus);
> +	pci_stop_root_bus(vmd->bus[VMD_BUS_0]);
>  	sysfs_remove_link(&vmd->dev->dev.kobj, "domain");
> -	pci_remove_root_bus(vmd->bus);
> +	pci_remove_root_bus(vmd->bus[VMD_BUS_0]);

[Severity: High]
This is a pre-existing issue, but does device removal or probe failure leak
IRQ resources?

In vmd_alloc_irqs(), vectors are successfully allocated:

vmd->msix_count = pci_alloc_irq_vectors(dev, vmd->first_vec + 1,
                                        vmd->msix_count, PCI_IRQ_MSIX);

However, they do not appear to be explicitly freed in error paths or upon
driver removal in vmd_remove(). The PCI subsystem guidelines state that every
error path or removal after successful allocation must call
pci_free_irq_vectors() to prevent resource leaks and potential exhaustion of
system IRQ capacity.

[Severity: High]
This is also a pre-existing issue, but is there a use-after-free risk during
teardown if a spurious interrupt fires?

In vmd_remove(), the SRCU structures are destroyed via vmd_cleanup_srcu().
However, the IRQ handlers are managed by devres (via devm_request_irq) and
will only be disabled after vmd_remove() returns.

If a spurious or shared interrupt fires after the SRCU structures are
destroyed but before devres cleanup completes, the handler will execute
and access the destroyed struct:

vmd_irq() {
    ...
    idx = srcu_read_lock(&irqs->srcu);
}

Could this race condition lead to memory corruption or a crash? Should the
interrupts be explicitly disabled before destroying the SRCU structures?

>  	/* CFGBAR is static, does not require releasing memory */
>  	kfree(vmd->resources[VMD_RES_MBAR_1].name);

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260625082940.1290740-1-szymon.durawa@linux.intel.com?part=6