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

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [High] The `id` parameter passed to PCI driver probe functions is now a pointer to a stack-allocated variable, breaking drivers that rely on pointer arithmetic to calculate indices or that cache the pointer.
--

commit 0cf7c75b08e81435f8eec9e2453a6dda8624a99b
Author: Gary Guo <[email protected]>
Subject: pci: fix UAF when probe runs concurrent to dyn ID removal

This commit modifies the PCI matching logic to copy the matched device ID
instead of returning a pointer to it. This ensures dynamic IDs are safely
handled without requiring the lock to be held during probe.

Link: https://lore.kernel.org/all/[email protected]/

> diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
> --- a/drivers/pci/pci-driver.c
> +++ b/drivers/pci/pci-driver.c

[ ... ]

> @@ -465,15 +470,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: High]
Does passing a pointer to the stack-allocated id variable here break drivers
that rely on the pointer belonging to their static id_table?

Some drivers compute an array index using pointer arithmetic against the
provided id pointer. For example, in drivers/char/agp/amd-k7-agp.c:

agp_amdk7_probe() {
    ...
    j = ent - agp_amdk7_pci_table;
    dev_info(&pdev->dev, "AMD %s chipset\n",
             amd_agp_device_ids[j].chipset_name);
    ...
}

Since the pointer now points to the stack frame in __pci_device_probe()
rather than the static table, won't this pointer subtraction yield a
wildly out-of-bounds index and cause a memory read fault during device
probing?

>  	}
>  	return error;
>  }

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/[email protected]?part=7