Re: [PATCH v2 1/2] PCI: Handle dev_set_name() failure in pci_setup_device()

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] The patch prevents a specific failure in `device_add()` (NULL name) but fails to address the systemic bug in `pci_device_add()` where `device_add()` failures are ignored, which leaves structurally invalid devices on `bus->devices` and continues to allow NULL pointer dereference panics.
--

--- Patch [1]: [PATCH v2 1/2] PCI: Handle dev_set_name() failure in pci_setup_device() ---
commit 91ceaf734e6165d4ec69baadee524cce3c4d990d
Author: Rihyeon Kim <rihyeon8648@gmail.com>

PCI: Handle dev_set_name() failure in pci_setup_device()

The commit adds error handling for dev_set_name() in pci_setup_device() to
prevent it from continuing with a NULL name. This avoids a later panic where
device_add() rejects the unnamed device but pci_device_add() ignores the
failure, leaving the invalid device on bus->devices with a freed dev->p.

Link: https://syzkaller.appspot.com/bug?extid=87bb32e345aa80c0554b

> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index dd0abbc63e18d..3e6f0eb7d456c 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
[ ... ]
> @@ -2052,9 +2053,13 @@ int pci_setup_device(struct pci_dev *dev)
>  	 */
>  	dev->msi_addr_mask = DMA_BIT_MASK(64);
>  
> -	dev_set_name(&dev->dev, "%04x:%02x:%02x.%d", pci_domain_nr(dev->bus),
> -		     dev->bus->number, PCI_SLOT(dev->devfn),
> -		     PCI_FUNC(dev->devfn));
> +	err = dev_set_name(&dev->dev, "%04x:%02x:%02x.%d",
> +			   pci_domain_nr(dev->bus), dev->bus->number,
> +			   PCI_SLOT(dev->devfn), PCI_FUNC(dev->devfn));
> +	if (err) {
> +		pci_release_of_node(dev);
> +		return err;
> +	}

[Severity: High]
This isn't a bug introduced by this patch, but while this change prevents
one trigger for the NULL pointer dereference, does it fully fix the
underlying vulnerability in pci_device_add()?

As the commit message describes, if device_add() fails, it frees dev->p
but pci_device_add() leaves the structurally invalid device on bus->devices,
which later causes __device_attach() to panic.

Since device_add() can fail for other reasons beyond an invalid name (such
as -ENOMEM or sysfs naming conflicts), won't those other failures still
trigger the exact same NULL pointer dereference?

In drivers/pci/probe.c:pci_device_add():

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

If device_add() fails here for a different reason, it only triggers a
warning while leaving the device on the list.

Should pci_device_add() also be updated to properly remove the device from
bus->devices when device_add() fails?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260726093200.146111-1-rihyeon8648@gmail.com?part=1