Re: [PATCH 3/3] pci: fix UAF when probe runs concurrent to dyn ID removal

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] Passing a pointer to a stack-allocated `struct pci_device_id` to driver `probe` functions introduces a stack use-after-free vulnerability for drivers that store the pointer for later use.
--

commit 2537209ca17dda70306c583f96a659580e419b4a
Author: Gary Guo <gary@garyguo.net>

pci: fix UAF when probe runs concurrent to dyn ID removal

Dynamic IDs are only guaranteed to be valid when dynids.lock is held, as
remove_id_store can free the node. This patch copies the matched ID in
pci_match_device to avoid the UAF.

Link: https://lore.kernel.org/all/20260619170503.518F61F00A3A@smtp.kernel.org/

> diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
> index 464ee52346fa8..2c76bba9a7156 100644
> --- a/drivers/pci/pci-driver.c
> +++ b/drivers/pci/pci-driver.c

[ ... ]

> @@ -474,15 +475,14 @@ void pci_probe_flush_workqueue(void)
>   */
>  static int __pci_device_probe(struct pci_driver *drv, struct pci_dev *pci_dev)
>  {
> -	const struct pci_device_id *id;
> +	struct pci_device_id id;
>  	int error = 0;
>  
>  	if (drv->probe) {
>  		error = -ENODEV;
>  
> -		id = pci_match_device(drv, pci_dev);
> -		if (id)
> -			error = pci_call_probe(drv, pci_dev, id);
> +		if (pci_match_device(drv, pci_dev, &id))
> +			error = pci_call_probe(drv, pci_dev, &id);

[Severity: Critical]
Could passing a pointer to the stack-allocated struct pci_device_id to the
driver's probe function introduce a stack use-after-free regression?

Some drivers, like nsp32 and ata_generic, save the pointer to the ID passed
into their probe function for later use. For example:

drivers/scsi/nsp32.c:nsp32_probe() {
    ...
    data->pci_devid   = id;
    ...
}

When normal operations later access this saved pointer, won't they dereference
invalid stack memory since the __pci_device_probe() stack frame has already
been destroyed? 

For example, when reading from procfs:

drivers/scsi/nsp32.c:nsp32_show_info() {
    ...
    model    = data->pci_devid->driver_data;
    ...
}

Similarly in ata_generic_init_one(), the id pointer is passed as host private
data for later use in ATA operations:

drivers/ata/ata_generic.c:ata_generic_init_one() {
    ...
    return ata_pci_bmdma_init_one(dev, ppi, &generic_sht, (void *)id, 0);
}

>  	}
>  	return error;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260626-pci_id_fix-v1-0-a35c803f1b95@garyguo.net?part=3