On Mon, 30 Mar 2026 19:38:41 +0200
"Danilo Krummrich" <[email protected]> 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/[email protected]/
>
> > } else if (action == BUS_NOTIFY_BOUND_DRIVER &&
> > pdev->is_virtfn && physfn == vdev->pdev) {
> > struct pci_driver *drv = pci_dev_driver(pdev);
