Skip to content

Commit 10a4206

Browse files
author
Danilo Krummrich
committed
PCI: use generic driver_override infrastructure
When a driver is probed through __driver_attach(), the bus' match() callback is called without the device lock held, thus accessing the driver_override field without a lock, which can cause a UAF. Fix this by using the driver-core driver_override infrastructure taking care of proper locking internally. Note that calling match() from __driver_attach() without the device lock held is intentional. [1] Link: https://lore.kernel.org/driver-core/DGRGTIRHA62X.3RY09D9SOK77P@kernel.org/ [1] Reported-by: Gui-Dong Han <hanguidong02@gmail.com> Closes: https://bugzilla.kernel.org/show_bug.cgi?id=220789 Fixes: 782a985 ("PCI: Introduce new device binding path using pci_dev.driver_override") Acked-by: Bjorn Helgaas <bhelgaas@google.com> Acked-by: Alex Williamson <alex@shazbot.org> Tested-by: Gui-Dong Han <hanguidong02@gmail.com> Reviewed-by: Gui-Dong Han <hanguidong02@gmail.com> Link: https://patch.msgid.link/20260324005919.2408620-6-dakr@kernel.org Signed-off-by: Danilo Krummrich <dakr@kernel.org>
1 parent 1cf996a commit 10a4206

6 files changed

Lines changed: 13 additions & 44 deletions

File tree

drivers/pci/pci-driver.c

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -138,9 +138,11 @@ static const struct pci_device_id *pci_match_device(struct pci_driver *drv,
138138
{
139139
struct pci_dynid *dynid;
140140
const struct pci_device_id *found_id = NULL, *ids;
141+
int ret;
141142

142143
/* When driver_override is set, only bind to the matching driver */
143-
if (dev->driver_override && strcmp(dev->driver_override, drv->name))
144+
ret = device_match_driver_override(&dev->dev, &drv->driver);
145+
if (ret == 0)
144146
return NULL;
145147

146148
/* Look at the dynamic ids first, before the static ones */
@@ -164,15 +166,15 @@ static const struct pci_device_id *pci_match_device(struct pci_driver *drv,
164166
* matching.
165167
*/
166168
if (found_id->override_only) {
167-
if (dev->driver_override)
169+
if (ret > 0)
168170
return found_id;
169171
} else {
170172
return found_id;
171173
}
172174
}
173175

174176
/* driver_override will always match, send a dummy id */
175-
if (dev->driver_override)
177+
if (ret > 0)
176178
return &pci_device_id_any;
177179
return NULL;
178180
}
@@ -452,7 +454,7 @@ static int __pci_device_probe(struct pci_driver *drv, struct pci_dev *pci_dev)
452454
static inline bool pci_device_can_probe(struct pci_dev *pdev)
453455
{
454456
return (!pdev->is_virtfn || pdev->physfn->sriov->drivers_autoprobe ||
455-
pdev->driver_override);
457+
device_has_driver_override(&pdev->dev));
456458
}
457459
#else
458460
static inline bool pci_device_can_probe(struct pci_dev *pdev)
@@ -1722,6 +1724,7 @@ static const struct cpumask *pci_device_irq_get_affinity(struct device *dev,
17221724

17231725
const struct bus_type pci_bus_type = {
17241726
.name = "pci",
1727+
.driver_override = true,
17251728
.match = pci_bus_match,
17261729
.uevent = pci_uevent,
17271730
.probe = pci_device_probe,

drivers/pci/pci-sysfs.c

Lines changed: 0 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -615,33 +615,6 @@ static ssize_t devspec_show(struct device *dev,
615615
static DEVICE_ATTR_RO(devspec);
616616
#endif
617617

618-
static ssize_t driver_override_store(struct device *dev,
619-
struct device_attribute *attr,
620-
const char *buf, size_t count)
621-
{
622-
struct pci_dev *pdev = to_pci_dev(dev);
623-
int ret;
624-
625-
ret = driver_set_override(dev, &pdev->driver_override, buf, count);
626-
if (ret)
627-
return ret;
628-
629-
return count;
630-
}
631-
632-
static ssize_t driver_override_show(struct device *dev,
633-
struct device_attribute *attr, char *buf)
634-
{
635-
struct pci_dev *pdev = to_pci_dev(dev);
636-
ssize_t len;
637-
638-
device_lock(dev);
639-
len = sysfs_emit(buf, "%s\n", pdev->driver_override);
640-
device_unlock(dev);
641-
return len;
642-
}
643-
static DEVICE_ATTR_RW(driver_override);
644-
645618
static struct attribute *pci_dev_attrs[] = {
646619
&dev_attr_power_state.attr,
647620
&dev_attr_resource.attr,
@@ -669,7 +642,6 @@ static struct attribute *pci_dev_attrs[] = {
669642
#ifdef CONFIG_OF
670643
&dev_attr_devspec.attr,
671644
#endif
672-
&dev_attr_driver_override.attr,
673645
&dev_attr_ari_enabled.attr,
674646
NULL,
675647
};

drivers/pci/probe.c

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2488,7 +2488,6 @@ static void pci_release_dev(struct device *dev)
24882488
pci_release_of_node(pci_dev);
24892489
pcibios_release_device(pci_dev);
24902490
pci_bus_put(pci_dev->bus);
2491-
kfree(pci_dev->driver_override);
24922491
bitmap_free(pci_dev->dma_alias_mask);
24932492
dev_dbg(dev, "device released\n");
24942493
kfree(pci_dev);

drivers/vfio/pci/vfio_pci_core.c

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1987,9 +1987,8 @@ static int vfio_pci_bus_notifier(struct notifier_block *nb,
19871987
pdev->is_virtfn && physfn == vdev->pdev) {
19881988
pci_info(vdev->pdev, "Captured SR-IOV VF %s driver_override\n",
19891989
pci_name(pdev));
1990-
pdev->driver_override = kasprintf(GFP_KERNEL, "%s",
1991-
vdev->vdev.ops->name);
1992-
WARN_ON(!pdev->driver_override);
1990+
WARN_ON(device_set_driver_override(&pdev->dev,
1991+
vdev->vdev.ops->name));
19931992
} else if (action == BUS_NOTIFY_BOUND_DRIVER &&
19941993
pdev->is_virtfn && physfn == vdev->pdev) {
19951994
struct pci_driver *drv = pci_dev_driver(pdev);

drivers/xen/xen-pciback/pci_stub.c

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -598,6 +598,8 @@ static int pcistub_seize(struct pci_dev *dev,
598598
return err;
599599
}
600600

601+
static struct pci_driver xen_pcibk_pci_driver;
602+
601603
/* Called when 'bind'. This means we must _NOT_ call pci_reset_function or
602604
* other functions that take the sysfs lock. */
603605
static int pcistub_probe(struct pci_dev *dev, const struct pci_device_id *id)
@@ -609,8 +611,8 @@ static int pcistub_probe(struct pci_dev *dev, const struct pci_device_id *id)
609611

610612
match = pcistub_match(dev);
611613

612-
if ((dev->driver_override &&
613-
!strcmp(dev->driver_override, PCISTUB_DRIVER_NAME)) ||
614+
if (device_match_driver_override(&dev->dev,
615+
&xen_pcibk_pci_driver.driver) > 0 ||
614616
match) {
615617

616618
if (dev->hdr_type != PCI_HEADER_TYPE_NORMAL

include/linux/pci.h

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -575,12 +575,6 @@ struct pci_dev {
575575
u8 supported_speeds; /* Supported Link Speeds Vector */
576576
phys_addr_t rom; /* Physical address if not from BAR */
577577
size_t romlen; /* Length if not from BAR */
578-
/*
579-
* Driver name to force a match. Do not set directly, because core
580-
* frees it. Use driver_set_override() to set or clear it.
581-
*/
582-
const char *driver_override;
583-
584578
unsigned long priv_flags; /* Private flags for the PCI driver */
585579

586580
/* These methods index pci_reset_fn_methods[] */

0 commit comments

Comments
 (0)