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.
Reported-by: Sashiko <[email protected]> Link: https://lore.kernel.org/all/[email protected]/ Fixes: 0994375e9614 ("PCI: add remove_id sysfs entry") Signed-off-by: Gary Guo <[email protected]> --- drivers/pci/pci-driver.c | 28 +++++++++++++++------------- include/linux/pci.h | 1 + 2 files changed, 16 insertions(+), 13 deletions(-) diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c index 2e80ae150ff4..4851061babcb 100644 --- a/drivers/pci/pci-driver.c +++ b/drivers/pci/pci-driver.c @@ -179,6 +179,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 @@ -186,9 +187,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; @@ -200,17 +201,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) @@ -466,12 +466,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); } @@ -1559,12 +1560,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; diff --git a/include/linux/pci.h b/include/linux/pci.h index 64b308b6e61c..92c17c116de6 100644 --- a/include/linux/pci.h +++ b/include/linux/pci.h @@ -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 -- 2.54.0
