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. Thus, make a copy in
pci_match_device(). Also, clarify that the id parameter is only valid
during probe.

Fixes: 0994375e96 ("PCI: add remove_id sysfs entry")
Reported-by: Sashiko <sashiko-bot@kernel.org>
Link: https://lore.kernel.org/all/20260619170503.518F61F00A3A@smtp.kernel.org/
Signed-off-by: Gary Guo <gary@garyguo.net>
Signed-off-by: Bjorn Helgaas <bhelgaas@google.com>
Reviewed-by: Danilo Krummrich <dakr@kernel.org>
Link: https://patch.msgid.link/20260723-pci_id_fix-v4-9-3580726844e1@garyguo.net
This commit is contained in:
Gary Guo 2026-07-23 23:00:48 +01:00 committed by Bjorn Helgaas
parent 04fde70f78
commit 3ffc4c9690
2 changed files with 16 additions and 13 deletions

View File

@ -180,6 +180,7 @@ static const struct pci_device_id pci_device_id_any = {
* pci_match_device - See if a device matches a driver's list of IDs
* @drv: the PCI driver to match against
* @dev: the PCI device structure to match against
* @id_copy: place to store copy of pci_device_id for dynamic ID
*
* Used by a driver to check whether a PCI device is in its list of
* supported devices or in the dynids list, which may have been augmented
@ -187,9 +188,9 @@ static const struct pci_device_id pci_device_id_any = {
* structure or %NULL if there is no match.
*/
static const struct pci_device_id *pci_match_device(struct pci_driver *drv,
struct pci_dev *dev)
struct pci_dev *dev,
struct pci_device_id *id_copy)
{
struct pci_dynid *dynid;
const struct pci_device_id *found_id = NULL;
struct pci_device_id dev_id;
int ret;
@ -201,17 +202,16 @@ static const struct pci_device_id *pci_match_device(struct pci_driver *drv,
dev_id = pci_id_from_device(dev);
/* Look at the dynamic ids first, before the static ones */
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 = &dynid->id;
break;
scoped_guard(spinlock, &drv->dynids.lock) {
struct pci_dynid *dynid;
list_for_each_entry(dynid, &drv->dynids.list, node) {
if (pci_match_one_id(&dynid->id, &dev_id)) {
*id_copy = dynid->id;
return id_copy;
}
}
}
spin_unlock(&drv->dynids.lock);
if (found_id)
return found_id;
found_id = do_pci_match_id(drv->id_table, &dev_id, ret > 0);
if (found_id)
@ -467,12 +467,13 @@ 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_copy;
int error = 0;
if (drv->probe) {
error = -ENODEV;
id = pci_match_device(drv, pci_dev);
id = pci_match_device(drv, pci_dev, &id_copy);
if (id)
error = pci_call_probe(drv, pci_dev, id);
}
@ -1560,12 +1561,13 @@ static int pci_bus_match(struct device *dev, const struct device_driver *drv)
struct pci_dev *pci_dev = to_pci_dev(dev);
struct pci_driver *pci_drv;
const struct pci_device_id *found_id;
struct pci_device_id id_copy;
if (pci_dev_binding_disallowed(pci_dev))
return 0;
pci_drv = (struct pci_driver *)to_pci_driver(drv);
found_id = pci_match_device(pci_drv, pci_dev);
found_id = pci_match_device(pci_drv, pci_dev, &id_copy);
if (found_id)
return 1;

View File

@ -979,6 +979,7 @@ struct module;
* function returns zero when the driver chooses to
* take "ownership" of the device or an error code
* (negative number) otherwise.
* The pci_device_id parameter is only valid during probe.
* The probe function always gets called from process
* context, so it can sleep.
* @remove: The remove() function gets called whenever a device