Re: [PATCH v4 8/9] PCI: Fix dyn_id add TOCTOU

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

Pre-existing issues:
- [High] `pci_match_device()` returns a pointer to the `id` field of a dyna=
mically allocated `struct pci_dynid` after releasing the `drv->dynids.lock`=
 spinlock, which can result in a Use-After-Free in the probe path if the ID=
 is concurrently removed via sysfs.
--

--- Patch [8]: [PATCH v4 8/9] PCI: Fix dyn_id add TOCTOU ---
commit b2b9cd8b65d097234cb7fc1e84df45480eb112de
Author: Gary Guo <[email protected]>

PCI: Fix dyn_id add TOCTOU

Currently there is a TOCTOU issue in new_id_store() as the dyn ID insertion
in pci_add_dynid() and the pci_match_device() are in separate critical
sections.

Fix this by moving the existing ID check to inside pci_add_dynid() and only
check against the static ID table outside the critical section.

> diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
> index c9424edb45481..ab3bb756ce89a 100644
> --- a/drivers/pci/pci-driver.c
> +++ b/drivers/pci/pci-driver.c
[ ... ]
> @@ -153,7 +190,7 @@ static const struct pci_device_id *pci_match_device(s=
truct pci_driver *drv,
>  						    struct pci_dev *dev)
>  {
>  	struct pci_dynid *dynid;
> -	const struct pci_device_id *found_id =3D NULL, *ids;
> +	const struct pci_device_id *found_id =3D NULL;

[Severity: High]
This isn't a bug introduced by this patch, but does pci_match_device() have=
 a
use-after-free risk when returning dynamic IDs?

It looks like it returns a pointer to the id field of a dynamically allocat=
ed
struct pci_dynid after releasing the drv->dynids.lock spinlock:

pci_match_device()
    spin_lock(&drv->dynids.lock);
    list_for_each_entry(dynid, &drv->dynids.list, node) {
        if (pci_match_one_id(&dynid->id, &dev_id)) {
            found_id =3D &dynid->id;
            break;
        }
    }
    spin_unlock(&drv->dynids.lock);

    if (found_id)
        return found_id;

If remove_id_store() is triggered via sysfs concurrently, it can acquire
dynids.lock, remove the dynamic ID, and call kfree() on the structure:

remove_id_store()
    list_del(&dynid->node);
    kfree(dynid);

If this happens while __pci_device_probe() has matched the dynamic ID but
hasn't finished executing the driver's probe() routine, could the driver
dereference a freed struct pci_device_id?

__pci_device_probe()
    id =3D pci_match_device(drv, pci_dev);
    if (id)
        error =3D pci_call_probe(drv, pci_dev, id);

>  	struct pci_device_id dev_id;
>  	int ret;

--=20
Sashiko AI review =C2=B7 https://sashiko.dev/#/patchset/20260723-pci_id_fix=
[email protected]?part=3D8