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

[David Jeffery]

On Sun, Jul 26, 2026 at 5:32 AM Rihyeon Kim <rihyeon8648@xxxxxxxxx> wrote:
>
> pci_setup_device() calls dev_set_name() but ignores its return value.
> dev_set_name() calls kobject_set_name_vargs(), which in turn allocates the
> name with kvasprintf_const() and can fail, leaving dev->dev.kobj.name NULL.
>
> Enumeration then continues with an unnamed device.  device_add() rejects
> it with -EINVAL because dev_name() returns NULL and pci_bus_type has no
> ->dev_name callback, but pci_device_add() only warns about that failure:
>
>   pci (null): [8086:2934] type 00 class 0x0c0300 conventional PCI endpoint
>   pci (null): BAR 4 [io  0xc0e0-0xc0ff]
>   ------------[ cut here ]------------
>   ret < 0
>   WARNING: drivers/pci/probe.c:2768 at pci_device_add+0x133b/0x1810
>   Call Trace:
>    pci_scan_single_device+0x1d0/0x240
>    pci_scan_slot+0x1c9/0x7c0
>    pci_scan_child_bus_extend+0x6b/0x7b0
>    pci_rescan_bus+0x18/0x40
>    rescan_store+0xfb/0x130
>
> The device was already added to bus->devices before device_add() ran and
> is not removed from it, so pci_bus_add_devices() later in the same
> pci_rescan_bus() call finds it and passes it to __device_attach(), which
> dereferences dev->p.  device_add() freed dev->p on its error path, so this
> is a NULL pointer dereference:
>
>   Oops: general protection fault, probably for non-canonical address ...
>   KASAN: null-ptr-deref in range [0x0000000000000108-0x000000000000010f]
>   RIP: 0010:__device_attach+0xb0/0x4d0
>   Call Trace:
>    device_initial_probe+0xaf/0xd0
>    pci_bus_add_device+0xc5/0x100
>    pci_bus_add_devices+0x9a/0x1f0
>    pci_rescan_bus+0x2a/0x40
>    rescan_store+0xfb/0x130
>   Kernel panic - not syncing: Fatal exception
>
> Check the return value and propagate the error instead.  pci_setup_device()
> already returns int, and both callers, pci_scan_device() and
> pci_iov_scan_device(), already release the device when it fails.  Release
> the OF node first, like the existing unknown header type path does, and
> update the kernel-doc for the new return value.
>
> Fixes: 1fa5ae857bb1 ("driver core: get rid of struct device's bus_id string array")
> Reported-by: syzbot+87bb32e345aa80c0554b@xxxxxxxxxxxxxxxxxxxxxxxxx
> Closes: https://syzkaller.appspot.com/bug?extid=87bb32e345aa80c0554b
> Tested-by: syzbot+87bb32e345aa80c0554b@xxxxxxxxxxxxxxxxxxxxxxxxx
> Reviewed-by: Krzysztof Wilczyński <kwilczynski@xxxxxxxxxx>
> Assisted-by: Claude:claude-opus-5 sparse
> Signed-off-by: Rihyeon Kim <rihyeon8648@xxxxxxxxx>
> ---
> The reproducer is the one from the syzbot report, unmodified.  Tested on
> 48a5a7ab8d6a with syzbot's config for this bug (x86_64, KASAN,
> CONFIG_FAILSLAB), under QEMU with "-machine q35 -usb" so that the
> 0000:00:1d.0 it removes and rescans exists.  Without the patch it gives the
> WARNING and then the panic quoted above; with it the allocation still fails
> in dev_set_name(), enumeration skips the device, and the device comes back
> on the next rescan.  syzbot tested it as well.
>
> The report also has an arm64 crash at 0000:00:01.0 with a virtio device, so
> this is not specific to that controller.
>
> v2:
>  - Fixes: points at 1fa5ae857bb1 rather than eebfcfb52ce7 (Krzysztof)
>  - update the kernel-doc for the new return value (Krzysztof)
>  - say kvasprintf_const() rather than kvasprintf() in the commit log
>    (Krzysztof)
>  - pick up Reviewed-by
>
>  drivers/pci/probe.c | 15 ++++++++++-----
>  1 file changed, 10 insertions(+), 5 deletions(-)
>
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index dd0abbc63e18..3e6f0eb7d456 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -2010,8 +2010,9 @@ static const char *pci_type_str(struct pci_dev *dev)
>   * Initialize the device structure with information about the device's
>   * vendor,class,memory and IO-space addresses, IRQ lines etc.
>   * Called at initialisation of the PCI subsystem and by CardBus services.
> - * Returns 0 on success and negative if unknown type of device (not normal,
> - * bridge or CardBus).
> + * Returns 0 on success, -ENOMEM if the device name could not be allocated,
> + * and -EIO if the device is of an unknown type (not normal, bridge or
> + * CardBus).
>   */
>  int pci_setup_device(struct pci_dev *dev)
>  {
> @@ -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;
> +       }
>
>         class = pci_class(dev);
>
> --
> 2.43.0
>

Looks good, thanks for doing this.

Reviewed-by: David Jeffery <djeffery@xxxxxxxxxx>