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: 782a985d7af2 ("PCI: Introduce new device binding path using pci_dev.driver_override")
Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
drivers/pci/pci-driver.c | 11 +++++++----
drivers/pci/pci-sysfs.c | 28 ----------------------------
drivers/pci/probe.c | 1 -
drivers/vfio/pci/vfio_pci_core.c | 5 ++---
drivers/xen/xen-pciback/pci_stub.c | 6 ++++--
include/linux/pci.h | 6 ------
6 files changed, 13 insertions(+), 44 deletions(-)
diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
index dd9075403987..d10ece0889f0 100644
--- a/drivers/pci/pci-driver.c
+++ b/drivers/pci/pci-driver.c
@@ -138,9 +138,11 @@ static const struct pci_device_id *pci_match_device(struct pci_driver *drv,
{
struct pci_dynid *dynid;
const struct pci_device_id *found_id = NULL, *ids;
+ int ret;
/* When driver_override is set, only bind to the matching driver */
- if (dev->driver_override && strcmp(dev->driver_override, drv->name))
+ ret = device_match_driver_override(&dev->dev, &drv->driver);
+ if (ret == 0)
return NULL;
/* Look at the dynamic ids first, before the static ones */
@@ -164,7 +166,7 @@ static const struct pci_device_id *pci_match_device(struct pci_driver *drv,
* matching.
*/
if (found_id->override_only) {
- if (dev->driver_override)
+ if (ret > 0)
return found_id;
} else {
return found_id;
@@ -172,7 +174,7 @@ static const struct pci_device_id *pci_match_device(struct pci_driver *drv,
}
/* driver_override will always match, send a dummy id */
- if (dev->driver_override)
+ if (ret > 0)
return &pci_device_id_any;
return NULL;
}
@@ -452,7 +454,7 @@ static int __pci_device_probe(struct pci_driver *drv, struct pci_dev *pci_dev)
static inline bool pci_device_can_probe(struct pci_dev *pdev)
{
return (!pdev->is_virtfn || pdev->physfn->sriov->drivers_autoprobe ||
- pdev->driver_override);
+ device_has_driver_override(&pdev->dev));
}
#else
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,
const struct bus_type pci_bus_type = {
.name = "pci",
+ .driver_override = true,
.match = pci_bus_match,
.uevent = pci_uevent,
.probe = pci_device_probe,
diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
index 16eaaf749ba9..a9006cf4e9c8 100644
--- a/drivers/pci/pci-sysfs.c
+++ b/drivers/pci/pci-sysfs.c
@@ -615,33 +615,6 @@ static ssize_t devspec_show(struct device *dev,
static DEVICE_ATTR_RO(devspec);
#endif
-static ssize_t driver_override_store(struct device *dev,
- struct device_attribute *attr,
- const char *buf, size_t count)
-{
- struct pci_dev *pdev = to_pci_dev(dev);
- int ret;
-
- ret = driver_set_override(dev, &pdev->driver_override, buf, count);
- if (ret)
- return ret;
-
- return count;
-}
-
-static ssize_t driver_override_show(struct device *dev,
- struct device_attribute *attr, char *buf)
-{
- struct pci_dev *pdev = to_pci_dev(dev);
- ssize_t len;
-
- device_lock(dev);
- len = sysfs_emit(buf, "%s\n", pdev->driver_override);
- device_unlock(dev);
- return len;
-}
-static DEVICE_ATTR_RW(driver_override);
-
static struct attribute *pci_dev_attrs[] = {
&dev_attr_power_state.attr,
&dev_attr_resource.attr,
@@ -669,7 +642,6 @@ static struct attribute *pci_dev_attrs[] = {
#ifdef CONFIG_OF
&dev_attr_devspec.attr,
#endif
- &dev_attr_driver_override.attr,
&dev_attr_ari_enabled.attr,
NULL,
};
diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
index bccc7a4bdd79..b4707640e102 100644
--- a/drivers/pci/probe.c
+++ b/drivers/pci/probe.c
@@ -2488,7 +2488,6 @@ static void pci_release_dev(struct device *dev)
pci_release_of_node(pci_dev);
pcibios_release_device(pci_dev);
pci_bus_put(pci_dev->bus);
- kfree(pci_dev->driver_override);
bitmap_free(pci_dev->dma_alias_mask);
dev_dbg(dev, "device released\n");
kfree(pci_dev);
diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c
index d43745fe4c84..460852f79f29 100644
--- a/drivers/vfio/pci/vfio_pci_core.c
+++ b/drivers/vfio/pci/vfio_pci_core.c
@@ -1987,9 +1987,8 @@ static int vfio_pci_bus_notifier(struct notifier_block *nb,
pdev->is_virtfn && physfn == vdev->pdev) {
pci_info(vdev->pdev, "Captured SR-IOV VF %s driver_override\n",
pci_name(pdev));
- pdev->driver_override = kasprintf(GFP_KERNEL, "%s",
- vdev->vdev.ops->name);
- WARN_ON(!pdev->driver_override);
+ WARN_ON(device_set_driver_override(&pdev->dev,
+ vdev->vdev.ops->name));
} else if (action == BUS_NOTIFY_BOUND_DRIVER &&
pdev->is_virtfn && physfn == vdev->pdev) {
struct pci_driver *drv = pci_dev_driver(pdev);
diff --git a/drivers/xen/xen-pciback/pci_stub.c b/drivers/xen/xen-pciback/pci_stub.c
index e4b27aecbf05..79a2b5dfd694 100644
--- a/drivers/xen/xen-pciback/pci_stub.c
+++ b/drivers/xen/xen-pciback/pci_stub.c
@@ -598,6 +598,8 @@ static int pcistub_seize(struct pci_dev *dev,
return err;
}
+static struct pci_driver xen_pcibk_pci_driver;
+
/* Called when 'bind'. This means we must _NOT_ call pci_reset_function or
* other functions that take the sysfs lock. */
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)
match = pcistub_match(dev);
- if ((dev->driver_override &&
- !strcmp(dev->driver_override, PCISTUB_DRIVER_NAME)) ||
+ if (device_match_driver_override(&dev->dev,
+ &xen_pcibk_pci_driver.driver) > 0 ||
match) {
if (dev->hdr_type != PCI_HEADER_TYPE_NORMAL
diff --git a/include/linux/pci.h b/include/linux/pci.h
index 1c270f1d5123..57e9463e4347 100644
--- a/include/linux/pci.h
+++ b/include/linux/pci.h
@@ -575,12 +575,6 @@ struct pci_dev {
u8 supported_speeds; /* Supported Link Speeds Vector */
phys_addr_t rom; /* Physical address if not from BAR */
size_t romlen; /* Length if not from BAR */
- /*
- * Driver name to force a match. Do not set directly, because core
- * frees it. Use driver_set_override() to set or clear it.
- */
- const char *driver_override;
-
unsigned long priv_flags; /* Private flags for the PCI driver */
/* These methods index pci_reset_fn_methods[] */
--
2.53.0(Cc: Jason)
On Tue Mar 24, 2026 at 1:59 AM CET, Danilo Krummrich wrote:
> diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c
> index d43745fe4c84..460852f79f29 100644
> --- a/drivers/vfio/pci/vfio_pci_core.c
> +++ b/drivers/vfio/pci/vfio_pci_core.c
> @@ -1987,9 +1987,8 @@ static int vfio_pci_bus_notifier(struct notifier_block *nb,
> pdev->is_virtfn && physfn == vdev->pdev) {
> pci_info(vdev->pdev, "Captured SR-IOV VF %s driver_override\n",
> pci_name(pdev));
> - pdev->driver_override = kasprintf(GFP_KERNEL, "%s",
> - vdev->vdev.ops->name);
> - WARN_ON(!pdev->driver_override);
> + WARN_ON(device_set_driver_override(&pdev->dev,
> + vdev->vdev.ops->name));
Technically, this is a change in behavior. If vdev->vdev.ops->name is NULL, it
will trigger the WARN_ON(), whereas before it would have just written "(null)"
into driver_override.
I assume that vfio_pci_core drivers are expected to set the name in struct
vfio_device_ops in the first place and this code (silently) relies on this
invariant?
Alex, Jason: Should we keep this hunk above as is and check for a proper name in
struct vfio_device_ops in vfio_pci_core_register_device() with a subsequent
patch?
> } else if (action == BUS_NOTIFY_BOUND_DRIVER &&
> pdev->is_virtfn && physfn == vdev->pdev) {
> struct pci_driver *drv = pci_dev_driver(pdev);
On Mon, 30 Mar 2026 19:38:41 +0200
"Danilo Krummrich" <dakr@kernel.org> wrote:
> (Cc: Jason)
>
> On Tue Mar 24, 2026 at 1:59 AM CET, Danilo Krummrich wrote:
> > diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c
> > index d43745fe4c84..460852f79f29 100644
> > --- a/drivers/vfio/pci/vfio_pci_core.c
> > +++ b/drivers/vfio/pci/vfio_pci_core.c
> > @@ -1987,9 +1987,8 @@ static int vfio_pci_bus_notifier(struct notifier_block *nb,
> > pdev->is_virtfn && physfn == vdev->pdev) {
> > pci_info(vdev->pdev, "Captured SR-IOV VF %s driver_override\n",
> > pci_name(pdev));
> > - pdev->driver_override = kasprintf(GFP_KERNEL, "%s",
> > - vdev->vdev.ops->name);
> > - WARN_ON(!pdev->driver_override);
> > + WARN_ON(device_set_driver_override(&pdev->dev,
> > + vdev->vdev.ops->name));
>
> Technically, this is a change in behavior. If vdev->vdev.ops->name is NULL, it
> will trigger the WARN_ON(), whereas before it would have just written "(null)"
> into driver_override.
It's worse than that. Looking at the implementation in [1], we have:
+static inline int device_set_driver_override(struct device *dev, const char *s)
+{
+ return __device_set_driver_override(dev, s, strlen(s));
+}
So if name is NULL, we oops in strlen() before we even hit the -EINVAL
and WARN_ON().
I don't believe we have any vfio-pci variant drivers where the name is
NULL, but kasprintf() handling NULL as "(null)" was a consideration in
this design, that even if there is no name the device is sequestered
with a driver_override that won't match an actual driver.
> I assume that vfio_pci_core drivers are expected to set the name in struct
> vfio_device_ops in the first place and this code (silently) relies on this
> invariant?
We do expect that, but it was previously safe either way to make sure
VFs are only bound to the same ops driver or barring that, at least
don't perform a standard driver match. The last thing we want to
happen automatically is for a user owned PF to create SR-IOV VFs that
automatically bind to native kernel drivers.
> Alex, Jason: Should we keep this hunk above as is and check for a proper name in
> struct vfio_device_ops in vfio_pci_core_register_device() with a subsequent
> patch?
Given the oops, my preference would be to roll it in here. This change
is what makes it a requirement that name cannot be NULL, where this was
safely handled with kasprintf(). Thanks,
Alex
[1] https://lore.kernel.org/all/20260302002729.19438-2-dakr@kernel.org/
>
> > } else if (action == BUS_NOTIFY_BOUND_DRIVER &&
> > pdev->is_virtfn && physfn == vdev->pdev) {
> > struct pci_driver *drv = pci_dev_driver(pdev);
On Mon Mar 30, 2026 at 10:10 PM CEST, Alex Williamson wrote:
> On Mon, 30 Mar 2026 19:38:41 +0200
> "Danilo Krummrich" <dakr@kernel.org> wrote:
>
>> (Cc: Jason)
>>
>> On Tue Mar 24, 2026 at 1:59 AM CET, Danilo Krummrich wrote:
>> > diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c
>> > index d43745fe4c84..460852f79f29 100644
>> > --- a/drivers/vfio/pci/vfio_pci_core.c
>> > +++ b/drivers/vfio/pci/vfio_pci_core.c
>> > @@ -1987,9 +1987,8 @@ static int vfio_pci_bus_notifier(struct notifier_block *nb,
>> > pdev->is_virtfn && physfn == vdev->pdev) {
>> > pci_info(vdev->pdev, "Captured SR-IOV VF %s driver_override\n",
>> > pci_name(pdev));
>> > - pdev->driver_override = kasprintf(GFP_KERNEL, "%s",
>> > - vdev->vdev.ops->name);
>> > - WARN_ON(!pdev->driver_override);
>> > + WARN_ON(device_set_driver_override(&pdev->dev,
>> > + vdev->vdev.ops->name));
>>
>> Technically, this is a change in behavior. If vdev->vdev.ops->name is NULL, it
>> will trigger the WARN_ON(), whereas before it would have just written "(null)"
>> into driver_override.
>
> It's worse than that. Looking at the implementation in [1], we have:
>
> +static inline int device_set_driver_override(struct device *dev, const char *s)
> +{
> + return __device_set_driver_override(dev, s, strlen(s));
> +}
>
> So if name is NULL, we oops in strlen() before we even hit the -EINVAL
> and WARN_ON().
This was changed in v2 [2] and the actual code in-tree is
static inline int device_set_driver_override(struct device *dev, const char *s)
{
return __device_set_driver_override(dev, s, s ? strlen(s) : 0);
}
so it does indeed return -EINVAL for a NULL pointer.
> I don't believe we have any vfio-pci variant drivers where the name is
> NULL, but kasprintf() handling NULL as "(null)" was a consideration in
> this design, that even if there is no name the device is sequestered
> with a driver_override that won't match an actual driver.
>
>> I assume that vfio_pci_core drivers are expected to set the name in struct
>> vfio_device_ops in the first place and this code (silently) relies on this
>> invariant?
>
> We do expect that, but it was previously safe either way to make sure
> VFs are only bound to the same ops driver or barring that, at least
> don't perform a standard driver match. The last thing we want to
> happen automatically is for a user owned PF to create SR-IOV VFs that
> automatically bind to native kernel drivers.
>
>> Alex, Jason: Should we keep this hunk above as is and check for a proper name in
>> struct vfio_device_ops in vfio_pci_core_register_device() with a subsequent
>> patch?
>
> Given the oops, my preference would be to roll it in here. This change
> is what makes it a requirement that name cannot be NULL, where this was
> safely handled with kasprintf().
Again, no oops here. :)
I still think it makes more sense to fail early in
vfio_pci_core_register_device(), rather than silently accept "(null)" in
driver_override. It also doesn't seem unreasonable with only the WARN_ON(), but
I can also just add vdev->vdev.ops->name ?: "(null)".
Please let me know what you prefer.
- Danilo
> [1] https://lore.kernel.org/all/20260302002729.19438-2-dakr@kernel.org/
[2] https://lore.kernel.org/driver-core/20260303115720.48783-1-dakr@kernel.org/
On Tue Mar 31, 2026 at 10:06 AM CEST, Danilo Krummrich wrote:
> On Mon Mar 30, 2026 at 10:10 PM CEST, Alex Williamson wrote:
>> On Mon, 30 Mar 2026 19:38:41 +0200
>> "Danilo Krummrich" <dakr@kernel.org> wrote:
>>
>>> (Cc: Jason)
>>>
>>> On Tue Mar 24, 2026 at 1:59 AM CET, Danilo Krummrich wrote:
>>> > diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c
>>> > index d43745fe4c84..460852f79f29 100644
>>> > --- a/drivers/vfio/pci/vfio_pci_core.c
>>> > +++ b/drivers/vfio/pci/vfio_pci_core.c
>>> > @@ -1987,9 +1987,8 @@ static int vfio_pci_bus_notifier(struct notifier_block *nb,
>>> > pdev->is_virtfn && physfn == vdev->pdev) {
>>> > pci_info(vdev->pdev, "Captured SR-IOV VF %s driver_override\n",
>>> > pci_name(pdev));
>>> > - pdev->driver_override = kasprintf(GFP_KERNEL, "%s",
>>> > - vdev->vdev.ops->name);
>>> > - WARN_ON(!pdev->driver_override);
>>> > + WARN_ON(device_set_driver_override(&pdev->dev,
>>> > + vdev->vdev.ops->name));
>>>
>>> Technically, this is a change in behavior. If vdev->vdev.ops->name is NULL, it
>>> will trigger the WARN_ON(), whereas before it would have just written "(null)"
>>> into driver_override.
>>
>> It's worse than that. Looking at the implementation in [1], we have:
>>
>> +static inline int device_set_driver_override(struct device *dev, const char *s)
>> +{
>> + return __device_set_driver_override(dev, s, strlen(s));
>> +}
>>
>> So if name is NULL, we oops in strlen() before we even hit the -EINVAL
>> and WARN_ON().
>
> This was changed in v2 [2] and the actual code in-tree is
>
> static inline int device_set_driver_override(struct device *dev, const char *s)
> {
> return __device_set_driver_override(dev, s, s ? strlen(s) : 0);
> }
>
> so it does indeed return -EINVAL for a NULL pointer.
>
>> I don't believe we have any vfio-pci variant drivers where the name is
>> NULL, but kasprintf() handling NULL as "(null)" was a consideration in
>> this design, that even if there is no name the device is sequestered
>> with a driver_override that won't match an actual driver.
>>
>>> I assume that vfio_pci_core drivers are expected to set the name in struct
>>> vfio_device_ops in the first place and this code (silently) relies on this
>>> invariant?
>>
>> We do expect that, but it was previously safe either way to make sure
>> VFs are only bound to the same ops driver or barring that, at least
>> don't perform a standard driver match. The last thing we want to
>> happen automatically is for a user owned PF to create SR-IOV VFs that
>> automatically bind to native kernel drivers.
>>
>>> Alex, Jason: Should we keep this hunk above as is and check for a proper name in
>>> struct vfio_device_ops in vfio_pci_core_register_device() with a subsequent
>>> patch?
>>
>> Given the oops, my preference would be to roll it in here. This change
>> is what makes it a requirement that name cannot be NULL, where this was
>> safely handled with kasprintf().
>
> Again, no oops here. :)
>
> I still think it makes more sense to fail early in
> vfio_pci_core_register_device(), rather than silently accept "(null)" in
> driver_override. It also doesn't seem unreasonable with only the WARN_ON(), but
> I can also just add vdev->vdev.ops->name ?: "(null)".
(Or just skip the call if !vdev->vdev.ops->name, as a user will read "(null)"
from sysfs either way.)
> Please let me know what you prefer.
>
> - Danilo
>
>> [1] https://lore.kernel.org/all/20260302002729.19438-2-dakr@kernel.org/
>
> [2] https://lore.kernel.org/driver-core/20260303115720.48783-1-dakr@kernel.org/
On Tue, 31 Mar 2026 10:22:14 +0200
"Danilo Krummrich" <dakr@kernel.org> wrote:
> On Tue Mar 31, 2026 at 10:06 AM CEST, Danilo Krummrich wrote:
> > On Mon Mar 30, 2026 at 10:10 PM CEST, Alex Williamson wrote:
> >> On Mon, 30 Mar 2026 19:38:41 +0200
> >> "Danilo Krummrich" <dakr@kernel.org> wrote:
> >>
> >>> (Cc: Jason)
> >>>
> >>> On Tue Mar 24, 2026 at 1:59 AM CET, Danilo Krummrich wrote:
> >>> > diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c
> >>> > index d43745fe4c84..460852f79f29 100644
> >>> > --- a/drivers/vfio/pci/vfio_pci_core.c
> >>> > +++ b/drivers/vfio/pci/vfio_pci_core.c
> >>> > @@ -1987,9 +1987,8 @@ static int vfio_pci_bus_notifier(struct notifier_block *nb,
> >>> > pdev->is_virtfn && physfn == vdev->pdev) {
> >>> > pci_info(vdev->pdev, "Captured SR-IOV VF %s driver_override\n",
> >>> > pci_name(pdev));
> >>> > - pdev->driver_override = kasprintf(GFP_KERNEL, "%s",
> >>> > - vdev->vdev.ops->name);
> >>> > - WARN_ON(!pdev->driver_override);
> >>> > + WARN_ON(device_set_driver_override(&pdev->dev,
> >>> > + vdev->vdev.ops->name));
> >>>
> >>> Technically, this is a change in behavior. If vdev->vdev.ops->name is NULL, it
> >>> will trigger the WARN_ON(), whereas before it would have just written "(null)"
> >>> into driver_override.
> >>
> >> It's worse than that. Looking at the implementation in [1], we have:
> >>
> >> +static inline int device_set_driver_override(struct device *dev, const char *s)
> >> +{
> >> + return __device_set_driver_override(dev, s, strlen(s));
> >> +}
> >>
> >> So if name is NULL, we oops in strlen() before we even hit the -EINVAL
> >> and WARN_ON().
> >
> > This was changed in v2 [2] and the actual code in-tree is
> >
> > static inline int device_set_driver_override(struct device *dev, const char *s)
> > {
> > return __device_set_driver_override(dev, s, s ? strlen(s) : 0);
> > }
> >
> > so it does indeed return -EINVAL for a NULL pointer.
Ok, good.
> >> I don't believe we have any vfio-pci variant drivers where the name is
> >> NULL, but kasprintf() handling NULL as "(null)" was a consideration in
> >> this design, that even if there is no name the device is sequestered
> >> with a driver_override that won't match an actual driver.
> >>
> >>> I assume that vfio_pci_core drivers are expected to set the name in struct
> >>> vfio_device_ops in the first place and this code (silently) relies on this
> >>> invariant?
> >>
> >> We do expect that, but it was previously safe either way to make sure
> >> VFs are only bound to the same ops driver or barring that, at least
> >> don't perform a standard driver match. The last thing we want to
> >> happen automatically is for a user owned PF to create SR-IOV VFs that
> >> automatically bind to native kernel drivers.
> >>
> >>> Alex, Jason: Should we keep this hunk above as is and check for a proper name in
> >>> struct vfio_device_ops in vfio_pci_core_register_device() with a subsequent
> >>> patch?
> >>
> >> Given the oops, my preference would be to roll it in here. This change
> >> is what makes it a requirement that name cannot be NULL, where this was
> >> safely handled with kasprintf().
> >
> > Again, no oops here. :)
> >
> > I still think it makes more sense to fail early in
> > vfio_pci_core_register_device(), rather than silently accept "(null)" in
> > driver_override. It also doesn't seem unreasonable with only the WARN_ON(), but
> > I can also just add vdev->vdev.ops->name ?: "(null)".
>
> (Or just skip the call if !vdev->vdev.ops->name, as a user will read "(null)"
> from sysfs either way.)
Hmm, I suppose they would, but there's a fundamental difference between
driver_override being set to "(null)" vs being NULL and only
interpreted as "(null)" via show. The former would prevent ID table
matching, the latter would not. The former is what was intended here,
without realizing both look the same through sysfs and present a
difficult debugging challenge.
Given no oops for strlen() now and no known in-kernel drivers with a
NULL name, I can post a patch that rejects a NULL name in the ops
structure in vfio_pci_core_register_device(), avoiding the entire
situation.
For this,
Acked-by: Alex Williamson <alex@shazbot.org>
Thanks,
Alex
On Tue, Mar 24, 2026 at 01:59:09AM +0100, Danilo Krummrich wrote:
> 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: 782a985d7af2 ("PCI: Introduce new device binding path using pci_dev.driver_override")
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>
> ---
> drivers/pci/pci-driver.c | 11 +++++++----
> drivers/pci/pci-sysfs.c | 28 ----------------------------
> drivers/pci/probe.c | 1 -
> include/linux/pci.h | 6 ------
For the above:
Acked-by: Bjorn Helgaas <bhelgaas@google.com>
"driver_override" is mentioned several places in
Documentation/ABI/testing/sysfs-bus-*. I assume this series doesn't
change the behavior documented there? Should any of this doc be
consolidated?
> drivers/vfio/pci/vfio_pci_core.c | 5 ++---
> drivers/xen/xen-pciback/pci_stub.c | 6 ++++--
> 6 files changed, 13 insertions(+), 44 deletions(-)
>
> diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
> index dd9075403987..d10ece0889f0 100644
> --- a/drivers/pci/pci-driver.c
> +++ b/drivers/pci/pci-driver.c
> @@ -138,9 +138,11 @@ static const struct pci_device_id *pci_match_device(struct pci_driver *drv,
> {
> struct pci_dynid *dynid;
> const struct pci_device_id *found_id = NULL, *ids;
> + int ret;
>
> /* When driver_override is set, only bind to the matching driver */
> - if (dev->driver_override && strcmp(dev->driver_override, drv->name))
> + ret = device_match_driver_override(&dev->dev, &drv->driver);
> + if (ret == 0)
> return NULL;
>
> /* Look at the dynamic ids first, before the static ones */
> @@ -164,7 +166,7 @@ static const struct pci_device_id *pci_match_device(struct pci_driver *drv,
> * matching.
> */
> if (found_id->override_only) {
> - if (dev->driver_override)
> + if (ret > 0)
> return found_id;
> } else {
> return found_id;
> @@ -172,7 +174,7 @@ static const struct pci_device_id *pci_match_device(struct pci_driver *drv,
> }
>
> /* driver_override will always match, send a dummy id */
> - if (dev->driver_override)
> + if (ret > 0)
> return &pci_device_id_any;
> return NULL;
> }
> @@ -452,7 +454,7 @@ static int __pci_device_probe(struct pci_driver *drv, struct pci_dev *pci_dev)
> static inline bool pci_device_can_probe(struct pci_dev *pdev)
> {
> return (!pdev->is_virtfn || pdev->physfn->sriov->drivers_autoprobe ||
> - pdev->driver_override);
> + device_has_driver_override(&pdev->dev));
> }
> #else
> 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,
>
> const struct bus_type pci_bus_type = {
> .name = "pci",
> + .driver_override = true,
> .match = pci_bus_match,
> .uevent = pci_uevent,
> .probe = pci_device_probe,
> diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
> index 16eaaf749ba9..a9006cf4e9c8 100644
> --- a/drivers/pci/pci-sysfs.c
> +++ b/drivers/pci/pci-sysfs.c
> @@ -615,33 +615,6 @@ static ssize_t devspec_show(struct device *dev,
> static DEVICE_ATTR_RO(devspec);
> #endif
>
> -static ssize_t driver_override_store(struct device *dev,
> - struct device_attribute *attr,
> - const char *buf, size_t count)
> -{
> - struct pci_dev *pdev = to_pci_dev(dev);
> - int ret;
> -
> - ret = driver_set_override(dev, &pdev->driver_override, buf, count);
> - if (ret)
> - return ret;
> -
> - return count;
> -}
> -
> -static ssize_t driver_override_show(struct device *dev,
> - struct device_attribute *attr, char *buf)
> -{
> - struct pci_dev *pdev = to_pci_dev(dev);
> - ssize_t len;
> -
> - device_lock(dev);
> - len = sysfs_emit(buf, "%s\n", pdev->driver_override);
> - device_unlock(dev);
> - return len;
> -}
> -static DEVICE_ATTR_RW(driver_override);
> -
> static struct attribute *pci_dev_attrs[] = {
> &dev_attr_power_state.attr,
> &dev_attr_resource.attr,
> @@ -669,7 +642,6 @@ static struct attribute *pci_dev_attrs[] = {
> #ifdef CONFIG_OF
> &dev_attr_devspec.attr,
> #endif
> - &dev_attr_driver_override.attr,
> &dev_attr_ari_enabled.attr,
> NULL,
> };
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index bccc7a4bdd79..b4707640e102 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -2488,7 +2488,6 @@ static void pci_release_dev(struct device *dev)
> pci_release_of_node(pci_dev);
> pcibios_release_device(pci_dev);
> pci_bus_put(pci_dev->bus);
> - kfree(pci_dev->driver_override);
> bitmap_free(pci_dev->dma_alias_mask);
> dev_dbg(dev, "device released\n");
> kfree(pci_dev);
> diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c
> index d43745fe4c84..460852f79f29 100644
> --- a/drivers/vfio/pci/vfio_pci_core.c
> +++ b/drivers/vfio/pci/vfio_pci_core.c
> @@ -1987,9 +1987,8 @@ static int vfio_pci_bus_notifier(struct notifier_block *nb,
> pdev->is_virtfn && physfn == vdev->pdev) {
> pci_info(vdev->pdev, "Captured SR-IOV VF %s driver_override\n",
> pci_name(pdev));
> - pdev->driver_override = kasprintf(GFP_KERNEL, "%s",
> - vdev->vdev.ops->name);
> - WARN_ON(!pdev->driver_override);
> + WARN_ON(device_set_driver_override(&pdev->dev,
> + vdev->vdev.ops->name));
> } else if (action == BUS_NOTIFY_BOUND_DRIVER &&
> pdev->is_virtfn && physfn == vdev->pdev) {
> struct pci_driver *drv = pci_dev_driver(pdev);
> diff --git a/drivers/xen/xen-pciback/pci_stub.c b/drivers/xen/xen-pciback/pci_stub.c
> index e4b27aecbf05..79a2b5dfd694 100644
> --- a/drivers/xen/xen-pciback/pci_stub.c
> +++ b/drivers/xen/xen-pciback/pci_stub.c
> @@ -598,6 +598,8 @@ static int pcistub_seize(struct pci_dev *dev,
> return err;
> }
>
> +static struct pci_driver xen_pcibk_pci_driver;
> +
> /* Called when 'bind'. This means we must _NOT_ call pci_reset_function or
> * other functions that take the sysfs lock. */
> 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)
>
> match = pcistub_match(dev);
>
> - if ((dev->driver_override &&
> - !strcmp(dev->driver_override, PCISTUB_DRIVER_NAME)) ||
> + if (device_match_driver_override(&dev->dev,
> + &xen_pcibk_pci_driver.driver) > 0 ||
> match) {
>
> if (dev->hdr_type != PCI_HEADER_TYPE_NORMAL
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 1c270f1d5123..57e9463e4347 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -575,12 +575,6 @@ struct pci_dev {
> u8 supported_speeds; /* Supported Link Speeds Vector */
> phys_addr_t rom; /* Physical address if not from BAR */
> size_t romlen; /* Length if not from BAR */
> - /*
> - * Driver name to force a match. Do not set directly, because core
> - * frees it. Use driver_set_override() to set or clear it.
> - */
> - const char *driver_override;
> -
> unsigned long priv_flags; /* Private flags for the PCI driver */
>
> /* These methods index pci_reset_fn_methods[] */
> --
> 2.53.0
>
On Thu Mar 26, 2026 at 7:08 PM CET, Bjorn Helgaas wrote:
> On Tue, Mar 24, 2026 at 01:59:09AM +0100, Danilo Krummrich wrote:
>> 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: 782a985d7af2 ("PCI: Introduce new device binding path using pci_dev.driver_override")
>> Signed-off-by: Danilo Krummrich <dakr@kernel.org>
>> ---
>> drivers/pci/pci-driver.c | 11 +++++++----
>> drivers/pci/pci-sysfs.c | 28 ----------------------------
>> drivers/pci/probe.c | 1 -
>> include/linux/pci.h | 6 ------
>
> For the above:
>
> Acked-by: Bjorn Helgaas <bhelgaas@google.com>
>
> "driver_override" is mentioned several places in
> Documentation/ABI/testing/sysfs-bus-*. I assume this series doesn't
> change the behavior documented there?
Correct, none of this is altered.
On Tue, Mar 24, 2026 at 9:00 AM Danilo Krummrich <dakr@kernel.org> wrote:
>
> 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: 782a985d7af2 ("PCI: Introduce new device binding path using pci_dev.driver_override")
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>
Tested on QEMU PCI with multiple debug configs enabled. The original
PoCs run cleanly without triggering the issue.
Thanks Danilo.
Tested-by: Gui-Dong Han <hanguidong02@gmail.com>
Reviewed-by: Gui-Dong Han <hanguidong02@gmail.com>
> ---
> drivers/pci/pci-driver.c | 11 +++++++----
> drivers/pci/pci-sysfs.c | 28 ----------------------------
> drivers/pci/probe.c | 1 -
> drivers/vfio/pci/vfio_pci_core.c | 5 ++---
> drivers/xen/xen-pciback/pci_stub.c | 6 ++++--
> include/linux/pci.h | 6 ------
> 6 files changed, 13 insertions(+), 44 deletions(-)
>
> diff --git a/drivers/pci/pci-driver.c b/drivers/pci/pci-driver.c
> index dd9075403987..d10ece0889f0 100644
> --- a/drivers/pci/pci-driver.c
> +++ b/drivers/pci/pci-driver.c
> @@ -138,9 +138,11 @@ static const struct pci_device_id *pci_match_device(struct pci_driver *drv,
> {
> struct pci_dynid *dynid;
> const struct pci_device_id *found_id = NULL, *ids;
> + int ret;
>
> /* When driver_override is set, only bind to the matching driver */
> - if (dev->driver_override && strcmp(dev->driver_override, drv->name))
> + ret = device_match_driver_override(&dev->dev, &drv->driver);
> + if (ret == 0)
> return NULL;
>
> /* Look at the dynamic ids first, before the static ones */
> @@ -164,7 +166,7 @@ static const struct pci_device_id *pci_match_device(struct pci_driver *drv,
> * matching.
> */
> if (found_id->override_only) {
> - if (dev->driver_override)
> + if (ret > 0)
> return found_id;
> } else {
> return found_id;
> @@ -172,7 +174,7 @@ static const struct pci_device_id *pci_match_device(struct pci_driver *drv,
> }
>
> /* driver_override will always match, send a dummy id */
> - if (dev->driver_override)
> + if (ret > 0)
> return &pci_device_id_any;
> return NULL;
> }
> @@ -452,7 +454,7 @@ static int __pci_device_probe(struct pci_driver *drv, struct pci_dev *pci_dev)
> static inline bool pci_device_can_probe(struct pci_dev *pdev)
> {
> return (!pdev->is_virtfn || pdev->physfn->sriov->drivers_autoprobe ||
> - pdev->driver_override);
> + device_has_driver_override(&pdev->dev));
> }
> #else
> 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,
>
> const struct bus_type pci_bus_type = {
> .name = "pci",
> + .driver_override = true,
> .match = pci_bus_match,
> .uevent = pci_uevent,
> .probe = pci_device_probe,
> diff --git a/drivers/pci/pci-sysfs.c b/drivers/pci/pci-sysfs.c
> index 16eaaf749ba9..a9006cf4e9c8 100644
> --- a/drivers/pci/pci-sysfs.c
> +++ b/drivers/pci/pci-sysfs.c
> @@ -615,33 +615,6 @@ static ssize_t devspec_show(struct device *dev,
> static DEVICE_ATTR_RO(devspec);
> #endif
>
> -static ssize_t driver_override_store(struct device *dev,
> - struct device_attribute *attr,
> - const char *buf, size_t count)
> -{
> - struct pci_dev *pdev = to_pci_dev(dev);
> - int ret;
> -
> - ret = driver_set_override(dev, &pdev->driver_override, buf, count);
> - if (ret)
> - return ret;
> -
> - return count;
> -}
> -
> -static ssize_t driver_override_show(struct device *dev,
> - struct device_attribute *attr, char *buf)
> -{
> - struct pci_dev *pdev = to_pci_dev(dev);
> - ssize_t len;
> -
> - device_lock(dev);
> - len = sysfs_emit(buf, "%s\n", pdev->driver_override);
> - device_unlock(dev);
> - return len;
> -}
> -static DEVICE_ATTR_RW(driver_override);
> -
> static struct attribute *pci_dev_attrs[] = {
> &dev_attr_power_state.attr,
> &dev_attr_resource.attr,
> @@ -669,7 +642,6 @@ static struct attribute *pci_dev_attrs[] = {
> #ifdef CONFIG_OF
> &dev_attr_devspec.attr,
> #endif
> - &dev_attr_driver_override.attr,
> &dev_attr_ari_enabled.attr,
> NULL,
> };
> diff --git a/drivers/pci/probe.c b/drivers/pci/probe.c
> index bccc7a4bdd79..b4707640e102 100644
> --- a/drivers/pci/probe.c
> +++ b/drivers/pci/probe.c
> @@ -2488,7 +2488,6 @@ static void pci_release_dev(struct device *dev)
> pci_release_of_node(pci_dev);
> pcibios_release_device(pci_dev);
> pci_bus_put(pci_dev->bus);
> - kfree(pci_dev->driver_override);
> bitmap_free(pci_dev->dma_alias_mask);
> dev_dbg(dev, "device released\n");
> kfree(pci_dev);
> diff --git a/drivers/vfio/pci/vfio_pci_core.c b/drivers/vfio/pci/vfio_pci_core.c
> index d43745fe4c84..460852f79f29 100644
> --- a/drivers/vfio/pci/vfio_pci_core.c
> +++ b/drivers/vfio/pci/vfio_pci_core.c
> @@ -1987,9 +1987,8 @@ static int vfio_pci_bus_notifier(struct notifier_block *nb,
> pdev->is_virtfn && physfn == vdev->pdev) {
> pci_info(vdev->pdev, "Captured SR-IOV VF %s driver_override\n",
> pci_name(pdev));
> - pdev->driver_override = kasprintf(GFP_KERNEL, "%s",
> - vdev->vdev.ops->name);
> - WARN_ON(!pdev->driver_override);
> + WARN_ON(device_set_driver_override(&pdev->dev,
> + vdev->vdev.ops->name));
> } else if (action == BUS_NOTIFY_BOUND_DRIVER &&
> pdev->is_virtfn && physfn == vdev->pdev) {
> struct pci_driver *drv = pci_dev_driver(pdev);
> diff --git a/drivers/xen/xen-pciback/pci_stub.c b/drivers/xen/xen-pciback/pci_stub.c
> index e4b27aecbf05..79a2b5dfd694 100644
> --- a/drivers/xen/xen-pciback/pci_stub.c
> +++ b/drivers/xen/xen-pciback/pci_stub.c
> @@ -598,6 +598,8 @@ static int pcistub_seize(struct pci_dev *dev,
> return err;
> }
>
> +static struct pci_driver xen_pcibk_pci_driver;
> +
> /* Called when 'bind'. This means we must _NOT_ call pci_reset_function or
> * other functions that take the sysfs lock. */
> 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)
>
> match = pcistub_match(dev);
>
> - if ((dev->driver_override &&
> - !strcmp(dev->driver_override, PCISTUB_DRIVER_NAME)) ||
> + if (device_match_driver_override(&dev->dev,
> + &xen_pcibk_pci_driver.driver) > 0 ||
> match) {
>
> if (dev->hdr_type != PCI_HEADER_TYPE_NORMAL
> diff --git a/include/linux/pci.h b/include/linux/pci.h
> index 1c270f1d5123..57e9463e4347 100644
> --- a/include/linux/pci.h
> +++ b/include/linux/pci.h
> @@ -575,12 +575,6 @@ struct pci_dev {
> u8 supported_speeds; /* Supported Link Speeds Vector */
> phys_addr_t rom; /* Physical address if not from BAR */
> size_t romlen; /* Length if not from BAR */
> - /*
> - * Driver name to force a match. Do not set directly, because core
> - * frees it. Use driver_set_override() to set or clear it.
> - */
> - const char *driver_override;
> -
> unsigned long priv_flags; /* Private flags for the PCI driver */
>
> /* These methods index pci_reset_fn_methods[] */
> --
> 2.53.0
>
© 2016 - 2026 Red Hat, Inc.