Re: [PATCH] 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] Unhandled device_add() failure in pci_device_add() leaves a corrupt device on bus->devices, triggering NULL pointer dereferences.
--

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

PCI: Handle dev_set_name() failure in pci_setup_device()

This commit checks the return value of dev_set_name() during
pci_setup_device() to prevent enumeration of unnamed devices. By
propagating the error early, it avoids a later failure in device_add()
that would trigger a NULL pointer dereference.

Reported-by: syzbot+87bb32e345aa80c0554b@syzkaller.appspotmail.com
Closes: https://syzkaller.appspot.com/bug?extid=87bb32e345aa80c0554b
Tested-by: syzbot+87bb32e345aa80c0554b@syzkaller.appspotmail.com

> Two things I noticed while working on this and deliberately left alone,
> since they look like separate changes:

[ ... ]

> - pci_device_add() ignores device_add() failure entirely; the WARN_ON() is
>   all there is and the half-added device stays on bus->devices.  I built a
>   kernel with [2] applied (it stops device_add() from freeing dev->p) and
>   the NULL dereference above does go away, but the WARNING and the stale
>   bus->devices entry remain.  Happy to follow up on that separately if you
>   think it is worth doing.

[Severity: High]
This is a pre-existing issue, but should the failure of device_add() in
pci_device_add() be handled with explicit error paths instead of just
emitting a warning?

When device_add() fails, it cleans up and frees dev->p:

drivers/pci/probe.c:pci_device_add() {
    ...
    ret = device_add(&dev->dev);
    WARN_ON(ret < 0);
    ...
}

Leaving the initialized device on the globally visible bus->devices list
allows pci_bus_add_devices() to iterate over it and pass it to
__device_attach().

Could this result in a kernel panic when __device_attach() unconditionally
dereferences the freed dev->p?

> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index dd0abbc63e18d..474c6cb327be5 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c

[ ... ]

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