Re: [PATCH v6 1/8] PCI: vmd: Add vmd_bus_enumeration() helper function

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

Pre-existing issues:
- [High] This problem wasn't introduced by this patch, but concurrent VMD driver probes will corrupt the global ACPI companion lookup hook, causing silent ACPI enumeration failures.
- [High] This problem wasn't introduced by this patch, but SRCU teardown in vmd_remove() races with active devm-managed IRQs, risking a Use-After-Free in the interrupt handler.
--

--- Patch [1]: [PATCH v6 1/8] PCI: vmd: Add vmd_bus_enumeration() helper function ---
commit 773d34aa9783fc8b62fb9cb40eac288e4cc56317
Author: Szymon Durawa <[email protected]>

PCI: vmd: Add vmd_bus_enumeration() helper function

Move the vmd bus enumeration code to a new helper vmd_bus_enumeration().
No functional changes.

Suggested-by: Nirmal Patel <[email protected]>
Signed-off-by: Szymon Durawa <[email protected]>

> diff --git a/drivers/pci/controller/vmd.c b/drivers/pci/controller/vmd.c
> index d4ae250d4bc6d..070b247b3fbbe 100644
> --- a/drivers/pci/controller/vmd.c
> +++ b/drivers/pci/controller/vmd.c
[ ... ]
> @@ -771,6 +771,54 @@ static int vmd_pm_enable_quirk(struct pci_dev *pdev, void *userdata)
>  	return 0;
>  }
>  
> +static void vmd_bus_enumeration(struct pci_bus *bus, unsigned long features)
> +{
> +	struct pci_bus *child;
> +	struct pci_dev *dev;
> +	int ret;
> +
> +	vmd_acpi_begin();

[Severity: High]
This isn't a bug introduced by this patch, but could concurrent VMD driver
probes corrupt the global ACPI companion lookup hook?

If two VMD devices are probed concurrently (e.g., via PCI hotplug or parallel
sysfs bind), one thread will successfully set the ACPI hook and set the global
static hook_installed = true.

The second thread gets -EBUSY from pci_acpi_set_companion_lookup_hook, returns
early from vmd_acpi_begin(), but hook_installed remains true.

When the first thread finishes bus enumeration and calls vmd_acpi_end(), it
clears the hook. The second thread, still enumerating its bus, will then fail
to find ACPI companions for its children, breaking features like power
management and IRQ routing.

> +
> +	pci_scan_child_bus(bus);
> +	vmd_domain_reset(vmd_from_bus(bus));

[ ... ]

[Severity: High]
This is also a pre-existing issue in this file, but does SRCU teardown in
vmd_remove() race with active devm-managed IRQs, risking a use-after-free in
the interrupt handler?

When the VMD driver is unbound (e.g., via sysfs) or removed via hotplug,
vmd_remove() calls vmd_cleanup_srcu(), which destroys the SRCU structs via
cleanup_srcu_struct().

However, because the driver's interrupts were requested with
devm_request_irq(), they are not freed until the devres release phase, which
executes after vmd_remove() returns.

If an interrupt fires between cleanup_srcu_struct() and devm_free_irq(), the
vmd_irq() handler will blindly invoke srcu_read_lock() on the freed SRCU
struct.

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