Re: [PATCH 4/5] PCI: Enable async shutdown support

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [Critical] In `pci_register_host_bridge()`, a failure in `device_register(&bus->dev)` leads to a double-free of the `bridge->dev` object.
- [High] In `pci_device_add()`, failure of `device_add()` is ignored, leaving a partially-initialized device in the subsystem lists.
--

commit 32aeb814f8bbaee48b0baf0572f23452cd1e50da
Author: David Jeffery <djeffery@redhat.com>

PCI: Enable async shutdown support

This commit adds asynchronous shutdown support for PCI devices. It aims to
reduce overall shutdown time in the same way async suspend does.

> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index dd0abbc63e18d..af23814464060 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -1046,6 +1046,7 @@ static int pci_register_host_bridge(struct pci_host_bridge *bridge)
>  
>  	bus->bridge = get_device(&bridge->dev);
>  	device_enable_async_suspend(bus->bridge);
> +	dev_set_async_shutdown(bus->bridge);

[Severity: Critical]
This is a pre-existing issue, and was not introduced by the patch under
review, but does the error handling in pci_register_host_bridge() lead to
a double-free of the bridge->dev object?

If device_register(&bus->dev) fails, the code jumps to the unregister label:

unregister:
	put_device(&bridge->dev);
	device_del(&bridge->dev);
free:
	...
	if (bus_registered)
		put_device(&bus->dev);

Because bus_registered is set to true right before checking the error from
device_register(), the free path calls put_device(&bus->dev).

Releasing the bus triggers release_pcibus_dev(), which implicitly drops the
pci_bus->bridge reference via put_device(pci_bus->bridge). Since the code
explicitly drops the reference at the unregister label as well, does this
drop the object's refcount to 0 prematurely?

[ ... ]

> @@ -2748,6 +2749,7 @@ void pci_device_add(struct pci_dev *dev, struct pci_bus *bus)
>  	pci_reassigndev_resource_alignment(dev);
>  
>  	pci_init_capabilities(dev);
> +	dev_set_async_shutdown(&dev->dev);
>  
>  	/*
>  	 * Add the device to our list of discovered devices

[Severity: High]
This isn't a bug introduced by this patch, but I noticed the failure of
device_add() is ignored later in pci_device_add():

	ret = device_add(&dev->dev);
	WARN_ON(ret < 0);

If device_add() fails, the device remains in the bus->devices list but is
not tracked by the driver core.

When the PCI device or bus is later removed, pci_destroy_dev() unconditionally
calls device_del(&dev->dev). Calling device_del() on a device that was never
successfully added via device_add() violates the driver core API.

Could this lead to a kernel crash or NULL pointer dereference when a PCI
device is enumerated under memory pressure?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260701135015.81937-1-djeffery@redhat.com?part=4