[PATCH v2 1/4] driver core: generalize driver_override in struct device

Danilo Krummrich posted 4 patches 1 month, 1 week ago
[PATCH v2 1/4] driver core: generalize driver_override in struct device
Posted by Danilo Krummrich 1 month, 1 week ago
Currently, there are 12 busses (including platform and PCI) that
duplicate the driver_override logic for their individual devices.

All of them seem to be prone to the bug described in [1].

While this could be solved for every bus individually using a separate
lock, solving this in the driver-core generically results in less (and
cleaner) changes overall.

Thus, move driver_override to struct device, provide corresponding
accessors for busses and handle locking with a separate lock internally.

In particular, add device_set_driver_override(),
device_has_driver_override(), device_match_driver_override() and a
helper, DEVICE_ATTR_DRIVER_OVERRIDE(), to declare the corresponding
sysfs store() and show() callbacks.

Until all busses have migrated, keep driver_set_override() in place.

Note that we can't use the device lock for the reasons described in [2].

Link: https://bugzilla.kernel.org/show_bug.cgi?id=220789 [1]
Link: https://lore.kernel.org/driver-core/DGRGTIRHA62X.3RY09D9SOK77P@kernel.org/ [2]
Co-developed-by: Gui-Dong Han <hanguidong02@gmail.com>
Signed-off-by: Gui-Dong Han <hanguidong02@gmail.com>
Signed-off-by: Danilo Krummrich <dakr@kernel.org>
---
 drivers/base/bus.c         | 43 ++++++++++++++++++++++++++-
 drivers/base/core.c        |  2 ++
 drivers/base/dd.c          | 60 ++++++++++++++++++++++++++++++++++++++
 include/linux/device.h     | 54 ++++++++++++++++++++++++++++++++++
 include/linux/device/bus.h |  4 +++
 5 files changed, 162 insertions(+), 1 deletion(-)

diff --git a/drivers/base/bus.c b/drivers/base/bus.c
index bb61d8adbab1..c734e7162b74 100644
--- a/drivers/base/bus.c
+++ b/drivers/base/bus.c
@@ -504,6 +504,36 @@ int bus_for_each_drv(const struct bus_type *bus, struct device_driver *start,
 }
 EXPORT_SYMBOL_GPL(bus_for_each_drv);
 
+static ssize_t driver_override_store(struct device *dev,
+				     struct device_attribute *attr,
+				     const char *buf, size_t count)
+{
+	int ret;
+
+	ret = __device_set_driver_override(dev, buf, count);
+	if (ret)
+		return ret;
+
+	return count;
+}
+
+static ssize_t driver_override_show(struct device *dev,
+				    struct device_attribute *attr, char *buf)
+{
+	guard(spinlock)(&dev->driver_override.lock);
+	return sysfs_emit(buf, "%s\n", dev->driver_override.name);
+}
+static DEVICE_ATTR_RW(driver_override);
+
+static struct attribute *driver_override_dev_attrs[] = {
+	&dev_attr_driver_override.attr,
+	NULL,
+};
+
+static const struct attribute_group driver_override_dev_group = {
+	.attrs = driver_override_dev_attrs,
+};
+
 /**
  * bus_add_device - add device to bus
  * @dev: device being added
@@ -537,9 +567,15 @@ int bus_add_device(struct device *dev)
 	if (error)
 		goto out_put;
 
+	if (sp->bus->driver_override) {
+		error = device_add_group(dev, &driver_override_dev_group);
+		if (error)
+			goto out_groups;
+	}
+
 	error = sysfs_create_link(&sp->devices_kset->kobj, &dev->kobj, dev_name(dev));
 	if (error)
-		goto out_groups;
+		goto out_override;
 
 	error = sysfs_create_link(&dev->kobj, &sp->subsys.kobj, "subsystem");
 	if (error)
@@ -550,6 +586,9 @@ int bus_add_device(struct device *dev)
 
 out_subsys:
 	sysfs_remove_link(&sp->devices_kset->kobj, dev_name(dev));
+out_override:
+	if (dev->bus->driver_override)
+		device_remove_group(dev, &driver_override_dev_group);
 out_groups:
 	device_remove_groups(dev, sp->bus->dev_groups);
 out_put:
@@ -607,6 +646,8 @@ void bus_remove_device(struct device *dev)
 
 	sysfs_remove_link(&dev->kobj, "subsystem");
 	sysfs_remove_link(&sp->devices_kset->kobj, dev_name(dev));
+	if (dev->bus->driver_override)
+		device_remove_group(dev, &driver_override_dev_group);
 	device_remove_groups(dev, dev->bus->dev_groups);
 	if (klist_node_attached(&dev->p->knode_bus))
 		klist_del(&dev->p->knode_bus);
diff --git a/drivers/base/core.c b/drivers/base/core.c
index 791f9e444df8..09b98f02f559 100644
--- a/drivers/base/core.c
+++ b/drivers/base/core.c
@@ -2556,6 +2556,7 @@ static void device_release(struct kobject *kobj)
 	devres_release_all(dev);
 
 	kfree(dev->dma_range_map);
+	kfree(dev->driver_override.name);
 
 	if (dev->release)
 		dev->release(dev);
@@ -3159,6 +3160,7 @@ void device_initialize(struct device *dev)
 	kobject_init(&dev->kobj, &device_ktype);
 	INIT_LIST_HEAD(&dev->dma_pools);
 	mutex_init(&dev->mutex);
+	spin_lock_init(&dev->driver_override.lock);
 	lockdep_set_novalidate_class(&dev->mutex);
 	spin_lock_init(&dev->devres_lock);
 	INIT_LIST_HEAD(&dev->devres_head);
diff --git a/drivers/base/dd.c b/drivers/base/dd.c
index 0354f209529c..697e36e63cab 100644
--- a/drivers/base/dd.c
+++ b/drivers/base/dd.c
@@ -381,6 +381,66 @@ static void __exit deferred_probe_exit(void)
 }
 __exitcall(deferred_probe_exit);
 
+int __device_set_driver_override(struct device *dev, const char *s, size_t len)
+{
+	const char *new, *old;
+	char *cp;
+
+	if (!s)
+		return -EINVAL;
+
+	/*
+	 * The stored value will be used in sysfs show callback (sysfs_emit()),
+	 * which has a length limit of PAGE_SIZE and adds a trailing newline.
+	 * Thus we can store one character less to avoid truncation during sysfs
+	 * show.
+	 */
+	if (len >= (PAGE_SIZE - 1))
+		return -EINVAL;
+
+	/*
+	 * Compute the real length of the string in case userspace sends us a
+	 * bunch of \0 characters like python likes to do.
+	 */
+	len = strlen(s);
+
+	if (!len) {
+		/* Empty string passed - clear override */
+		spin_lock(&dev->driver_override.lock);
+		old = dev->driver_override.name;
+		dev->driver_override.name = NULL;
+		spin_unlock(&dev->driver_override.lock);
+		kfree(old);
+
+		return 0;
+	}
+
+	cp = strnchr(s, len, '\n');
+	if (cp)
+		len = cp - s;
+
+	new = kstrndup(s, len, GFP_KERNEL);
+	if (!new)
+		return -ENOMEM;
+
+	spin_lock(&dev->driver_override.lock);
+	old = dev->driver_override.name;
+	if (cp != s) {
+		dev->driver_override.name = new;
+		spin_unlock(&dev->driver_override.lock);
+	} else {
+		/* "\n" passed - clear override */
+		dev->driver_override.name = NULL;
+		spin_unlock(&dev->driver_override.lock);
+
+		kfree(new);
+	}
+	kfree(old);
+
+	return 0;
+}
+EXPORT_SYMBOL_GPL(__device_set_driver_override);
+
 /**
  * device_is_bound() - Check if device is bound to a driver
  * @dev: device to check
diff --git a/include/linux/device.h b/include/linux/device.h
index 0be95294b6e6..e65d564f01cd 100644
--- a/include/linux/device.h
+++ b/include/linux/device.h
@@ -483,6 +483,8 @@ struct device_physical_location {
  * 		on.  This shrinks the "Board Support Packages" (BSPs) and
  * 		minimizes board-specific #ifdefs in drivers.
  * @driver_data: Private pointer for driver specific info.
+ * @driver_override: Driver name to force a match.  Do not touch directly; use
+ *		     device_set_driver_override() instead.
  * @links:	Links to suppliers and consumers of this device.
  * @power:	For device power management.
  *		See Documentation/driver-api/pm/devices.rst for details.
@@ -576,6 +578,10 @@ struct device {
 					   core doesn't touch it */
 	void		*driver_data;	/* Driver data, set and get with
 					   dev_set_drvdata/dev_get_drvdata */
+	struct {
+		const char	*name;
+		spinlock_t	lock;
+	} driver_override;
 	struct mutex		mutex;	/* mutex to synchronize calls to
 					 * its driver.
 					 */
@@ -701,6 +707,54 @@ struct device_link {
 
 #define kobj_to_dev(__kobj)	container_of_const(__kobj, struct device, kobj)
 
+int __device_set_driver_override(struct device *dev, const char *s, size_t len);
+
+/**
+ * device_set_driver_override() - Helper to set or clear driver override.
+ * @dev: Device to change
+ * @s: NUL-terminated string, new driver name to force a match, pass empty
+ *     string to clear it ("" or "\n", where the latter is only for sysfs
+ *     interface).
+ *
+ * Helper to set or clear driver override of a device.
+ *
+ * Returns: 0 on success or a negative error code on failure.
+ */
+static inline int device_set_driver_override(struct device *dev, const char *s)
+{
+	return __device_set_driver_override(dev, s, s ? strlen(s) : 0);
+}
+
+/**
+ * device_has_driver_override() - Check if a driver override has been set.
+ * @dev: device to check
+ *
+ * Returns true if a driver override has been set for this device.
+ */
+static inline bool device_has_driver_override(struct device *dev)
+{
+	guard(spinlock)(&dev->driver_override.lock);
+	return !!dev->driver_override.name;
+}
+
+/**
+ * device_match_driver_override() - Match a driver against the device's driver_override.
+ * @dev: device to check
+ * @drv: driver to match against
+ *
+ * Returns > 0 if a driver override is set and matches the given driver, 0 if a
+ * driver override is set but does not match, or < 0 if a driver override is not
+ * set at all.
+ */
+static inline int device_match_driver_override(struct device *dev,
+					       const struct device_driver *drv)
+{
+	guard(spinlock)(&dev->driver_override.lock);
+	if (dev->driver_override.name)
+		return !strcmp(dev->driver_override.name, drv->name);
+	return -1;
+}
+
 /**
  * device_iommu_mapped - Returns true when the device DMA is translated
  *			 by an IOMMU
diff --git a/include/linux/device/bus.h b/include/linux/device/bus.h
index 99c3c83ea520..cda597812324 100644
--- a/include/linux/device/bus.h
+++ b/include/linux/device/bus.h
@@ -63,6 +63,9 @@ struct fwnode_handle;
  *			this bus.
  * @pm:		Power management operations of this bus, callback the specific
  *		device driver's pm-ops.
+ * @driver_override:	Set to true if this bus supports the driver_override
+ *			mechanism, which allows userspace to force a specific
+ *			driver to bind to a device via a sysfs attribute.
  * @need_parent_lock:	When probing or removing a device on this bus, the
  *			device core should lock the device's parent.
  *
@@ -104,6 +107,7 @@ struct bus_type {
 
 	const struct dev_pm_ops *pm;
 
+	bool driver_override;
 	bool need_parent_lock;
 };
 
-- 
2.53.0
Re: [PATCH v2 1/4] driver core: generalize driver_override in struct device
Posted by Gui-Dong Han 1 month ago
On Tue, Mar 3, 2026 at 7:57 PM Danilo Krummrich <dakr@kernel.org> wrote:
>
> Currently, there are 12 busses (including platform and PCI) that
> duplicate the driver_override logic for their individual devices.
>
> All of them seem to be prone to the bug described in [1].
>
> While this could be solved for every bus individually using a separate
> lock, solving this in the driver-core generically results in less (and
> cleaner) changes overall.
>
> Thus, move driver_override to struct device, provide corresponding
> accessors for busses and handle locking with a separate lock internally.
>
> In particular, add device_set_driver_override(),
> device_has_driver_override(), device_match_driver_override() and a
> helper, DEVICE_ATTR_DRIVER_OVERRIDE(), to declare the corresponding
> sysfs store() and show() callbacks.
>
> Until all busses have migrated, keep driver_set_override() in place.
>
> Note that we can't use the device lock for the reasons described in [2].
>
> Link: https://bugzilla.kernel.org/show_bug.cgi?id=220789 [1]
> Link: https://lore.kernel.org/driver-core/DGRGTIRHA62X.3RY09D9SOK77P@kernel.org/ [2]
> Co-developed-by: Gui-Dong Han <hanguidong02@gmail.com>
> Signed-off-by: Gui-Dong Han <hanguidong02@gmail.com>
> Signed-off-by: Danilo Krummrich <dakr@kernel.org>

Tested this on QEMU PCI with various debug options enabled.
Everything looks good so far, no issues found.

Will keep testing.

> ---
>  drivers/base/bus.c         | 43 ++++++++++++++++++++++++++-
>  drivers/base/core.c        |  2 ++
>  drivers/base/dd.c          | 60 ++++++++++++++++++++++++++++++++++++++
>  include/linux/device.h     | 54 ++++++++++++++++++++++++++++++++++
>  include/linux/device/bus.h |  4 +++
>  5 files changed, 162 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/base/bus.c b/drivers/base/bus.c
> index bb61d8adbab1..c734e7162b74 100644
> --- a/drivers/base/bus.c
> +++ b/drivers/base/bus.c
> @@ -504,6 +504,36 @@ int bus_for_each_drv(const struct bus_type *bus, struct device_driver *start,
>  }
>  EXPORT_SYMBOL_GPL(bus_for_each_drv);
>
> +static ssize_t driver_override_store(struct device *dev,
> +                                    struct device_attribute *attr,
> +                                    const char *buf, size_t count)
> +{
> +       int ret;
> +
> +       ret = __device_set_driver_override(dev, buf, count);
> +       if (ret)
> +               return ret;
> +
> +       return count;
> +}
> +
> +static ssize_t driver_override_show(struct device *dev,
> +                                   struct device_attribute *attr, char *buf)
> +{
> +       guard(spinlock)(&dev->driver_override.lock);
> +       return sysfs_emit(buf, "%s\n", dev->driver_override.name);
> +}
> +static DEVICE_ATTR_RW(driver_override);
> +
> +static struct attribute *driver_override_dev_attrs[] = {
> +       &dev_attr_driver_override.attr,
> +       NULL,
> +};
> +
> +static const struct attribute_group driver_override_dev_group = {
> +       .attrs = driver_override_dev_attrs,
> +};
> +
>  /**
>   * bus_add_device - add device to bus
>   * @dev: device being added
> @@ -537,9 +567,15 @@ int bus_add_device(struct device *dev)
>         if (error)
>                 goto out_put;
>
> +       if (sp->bus->driver_override) {
> +               error = device_add_group(dev, &driver_override_dev_group);
> +               if (error)
> +                       goto out_groups;
> +       }
> +
>         error = sysfs_create_link(&sp->devices_kset->kobj, &dev->kobj, dev_name(dev));
>         if (error)
> -               goto out_groups;
> +               goto out_override;
>
>         error = sysfs_create_link(&dev->kobj, &sp->subsys.kobj, "subsystem");
>         if (error)
> @@ -550,6 +586,9 @@ int bus_add_device(struct device *dev)
>
>  out_subsys:
>         sysfs_remove_link(&sp->devices_kset->kobj, dev_name(dev));
> +out_override:
> +       if (dev->bus->driver_override)
> +               device_remove_group(dev, &driver_override_dev_group);
>  out_groups:
>         device_remove_groups(dev, sp->bus->dev_groups);
>  out_put:
> @@ -607,6 +646,8 @@ void bus_remove_device(struct device *dev)
>
>         sysfs_remove_link(&dev->kobj, "subsystem");
>         sysfs_remove_link(&sp->devices_kset->kobj, dev_name(dev));
> +       if (dev->bus->driver_override)
> +               device_remove_group(dev, &driver_override_dev_group);
>         device_remove_groups(dev, dev->bus->dev_groups);
>         if (klist_node_attached(&dev->p->knode_bus))
>                 klist_del(&dev->p->knode_bus);
> diff --git a/drivers/base/core.c b/drivers/base/core.c
> index 791f9e444df8..09b98f02f559 100644
> --- a/drivers/base/core.c
> +++ b/drivers/base/core.c
> @@ -2556,6 +2556,7 @@ static void device_release(struct kobject *kobj)
>         devres_release_all(dev);
>
>         kfree(dev->dma_range_map);
> +       kfree(dev->driver_override.name);
>
>         if (dev->release)
>                 dev->release(dev);
> @@ -3159,6 +3160,7 @@ void device_initialize(struct device *dev)
>         kobject_init(&dev->kobj, &device_ktype);
>         INIT_LIST_HEAD(&dev->dma_pools);
>         mutex_init(&dev->mutex);
> +       spin_lock_init(&dev->driver_override.lock);
>         lockdep_set_novalidate_class(&dev->mutex);
>         spin_lock_init(&dev->devres_lock);
>         INIT_LIST_HEAD(&dev->devres_head);
> diff --git a/drivers/base/dd.c b/drivers/base/dd.c
> index 0354f209529c..697e36e63cab 100644
> --- a/drivers/base/dd.c
> +++ b/drivers/base/dd.c
> @@ -381,6 +381,66 @@ static void __exit deferred_probe_exit(void)
>  }
>  __exitcall(deferred_probe_exit);
>
> +int __device_set_driver_override(struct device *dev, const char *s, size_t len)
> +{
> +       const char *new, *old;
> +       char *cp;
> +
> +       if (!s)
> +               return -EINVAL;
> +
> +       /*
> +        * The stored value will be used in sysfs show callback (sysfs_emit()),
> +        * which has a length limit of PAGE_SIZE and adds a trailing newline.
> +        * Thus we can store one character less to avoid truncation during sysfs
> +        * show.
> +        */
> +       if (len >= (PAGE_SIZE - 1))
> +               return -EINVAL;
> +
> +       /*
> +        * Compute the real length of the string in case userspace sends us a
> +        * bunch of \0 characters like python likes to do.
> +        */
> +       len = strlen(s);
> +
> +       if (!len) {
> +               /* Empty string passed - clear override */
> +               spin_lock(&dev->driver_override.lock);
> +               old = dev->driver_override.name;
> +               dev->driver_override.name = NULL;
> +               spin_unlock(&dev->driver_override.lock);
> +               kfree(old);
> +
> +               return 0;
> +       }
> +
> +       cp = strnchr(s, len, '\n');
> +       if (cp)
> +               len = cp - s;
> +
> +       new = kstrndup(s, len, GFP_KERNEL);
> +       if (!new)
> +               return -ENOMEM;
> +
> +       spin_lock(&dev->driver_override.lock);
> +       old = dev->driver_override.name;
> +       if (cp != s) {
> +               dev->driver_override.name = new;
> +               spin_unlock(&dev->driver_override.lock);
> +       } else {
> +               /* "\n" passed - clear override */
> +               dev->driver_override.name = NULL;
> +               spin_unlock(&dev->driver_override.lock);
> +
> +               kfree(new);
> +       }
> +       kfree(old);
> +
> +       return 0;
> +}
> +EXPORT_SYMBOL_GPL(__device_set_driver_override);
> +
>  /**
>   * device_is_bound() - Check if device is bound to a driver
>   * @dev: device to check
> diff --git a/include/linux/device.h b/include/linux/device.h
> index 0be95294b6e6..e65d564f01cd 100644
> --- a/include/linux/device.h
> +++ b/include/linux/device.h
> @@ -483,6 +483,8 @@ struct device_physical_location {
>   *             on.  This shrinks the "Board Support Packages" (BSPs) and
>   *             minimizes board-specific #ifdefs in drivers.
>   * @driver_data: Private pointer for driver specific info.
> + * @driver_override: Driver name to force a match.  Do not touch directly; use
> + *                  device_set_driver_override() instead.
>   * @links:     Links to suppliers and consumers of this device.
>   * @power:     For device power management.
>   *             See Documentation/driver-api/pm/devices.rst for details.
> @@ -576,6 +578,10 @@ struct device {
>                                            core doesn't touch it */
>         void            *driver_data;   /* Driver data, set and get with
>                                            dev_set_drvdata/dev_get_drvdata */
> +       struct {
> +               const char      *name;
> +               spinlock_t      lock;
> +       } driver_override;
>         struct mutex            mutex;  /* mutex to synchronize calls to
>                                          * its driver.
>                                          */
> @@ -701,6 +707,54 @@ struct device_link {
>
>  #define kobj_to_dev(__kobj)    container_of_const(__kobj, struct device, kobj)
>
> +int __device_set_driver_override(struct device *dev, const char *s, size_t len);
> +
> +/**
> + * device_set_driver_override() - Helper to set or clear driver override.
> + * @dev: Device to change
> + * @s: NUL-terminated string, new driver name to force a match, pass empty
> + *     string to clear it ("" or "\n", where the latter is only for sysfs
> + *     interface).
> + *
> + * Helper to set or clear driver override of a device.
> + *
> + * Returns: 0 on success or a negative error code on failure.
> + */
> +static inline int device_set_driver_override(struct device *dev, const char *s)
> +{
> +       return __device_set_driver_override(dev, s, s ? strlen(s) : 0);
> +}
> +
> +/**
> + * device_has_driver_override() - Check if a driver override has been set.
> + * @dev: device to check
> + *
> + * Returns true if a driver override has been set for this device.
> + */
> +static inline bool device_has_driver_override(struct device *dev)
> +{
> +       guard(spinlock)(&dev->driver_override.lock);
> +       return !!dev->driver_override.name;
> +}
> +
> +/**
> + * device_match_driver_override() - Match a driver against the device's driver_override.
> + * @dev: device to check
> + * @drv: driver to match against
> + *
> + * Returns > 0 if a driver override is set and matches the given driver, 0 if a
> + * driver override is set but does not match, or < 0 if a driver override is not
> + * set at all.
> + */
> +static inline int device_match_driver_override(struct device *dev,
> +                                              const struct device_driver *drv)
> +{
> +       guard(spinlock)(&dev->driver_override.lock);
> +       if (dev->driver_override.name)
> +               return !strcmp(dev->driver_override.name, drv->name);
> +       return -1;
> +}
> +
>  /**
>   * device_iommu_mapped - Returns true when the device DMA is translated
>   *                      by an IOMMU
> diff --git a/include/linux/device/bus.h b/include/linux/device/bus.h
> index 99c3c83ea520..cda597812324 100644
> --- a/include/linux/device/bus.h
> +++ b/include/linux/device/bus.h
> @@ -63,6 +63,9 @@ struct fwnode_handle;
>   *                     this bus.
>   * @pm:                Power management operations of this bus, callback the specific
>   *             device driver's pm-ops.
> + * @driver_override:   Set to true if this bus supports the driver_override
> + *                     mechanism, which allows userspace to force a specific
> + *                     driver to bind to a device via a sysfs attribute.
>   * @need_parent_lock:  When probing or removing a device on this bus, the
>   *                     device core should lock the device's parent.
>   *
> @@ -104,6 +107,7 @@ struct bus_type {
>
>         const struct dev_pm_ops *pm;
>
> +       bool driver_override;
>         bool need_parent_lock;
>  };
>
> --
> 2.53.0
>
Re: [PATCH v2 1/4] driver core: generalize driver_override in struct device
Posted by Frank Li 1 month ago
From: Frank Li (AI-BOT) <frank.li@nxp.com>

AI bot review and may be useless.

> +static ssize_t driver_override_store(struct device *dev,
> +				     struct device_attribute *attr,
> +				     const char *buf, size_t count)
> +{
> +	int ret;
> +
> +	ret = __device_set_driver_override(dev, buf, count);
> +	if (ret)
> +		return ret;
> +
> +	return count;
> +}

Potential issue: if __device_set_driver_override() succeeds but
modifies state, returning 'count' may not reflect actual bytes
consumed if buf contains embedded nulls. Consider documenting
expected behavior or validating input length upfront.

> +static ssize_t driver_override_show(struct device *dev,
> +				    struct device_attribute *attr, char *buf)
> +{
> +	guard(spinlock)(&dev->driver_override.lock);
> +	return sysfs_emit(buf, "%s\n", dev->driver_override.name);
> +}

Potential NULL dereference: if dev->driver_override.name is NULL,
sysfs_emit() will print "(null)". Add explicit check or document
that NULL is acceptable output.

> +	if (sp->bus->driver_override) {
> +		error = device_add_group(dev, &driver_override_dev_group);
> +		if (error)
> +			goto out_groups;
> +	}

Error path bug: on device_add_group() failure, code jumps to
out_groups but should jump to out_override (which doesn't exist
yet). This leaks the sysfs link created below. Reorder or add
intermediate label.

> +	error = sysfs_create_link(&sp->devices_kset->kobj, &dev->kobj, dev_name(dev));
> +	if (error)
> -		goto out_groups;
> +		goto out_override;

Good: label renamed to match new cleanup order.

> +out_override:
> +	if (dev->bus->driver_override)
> +		device_remove_group(dev, &driver_override_dev_group);

Potential issue: dev->bus may be NULL or different from sp->bus at
this point. Use sp->bus for consistency with the add path above.

> +int __device_set_driver_override(struct device *dev, const char *s, size_t len)
> +{
> +	const char *new, *old;
> +	char *cp;
> +
> +	if (!s)
> +		return -EINVAL;

Inconsistency: len parameter is passed but then recalculated via
strlen(s) below. If len is meant to be trusted, don't recalculate.
If not, remove the parameter or document why both exist.

> +	len = strlen(s);

This overwrites the len parameter, making the initial bounds check
at PAGE_SIZE potentially useless. Clarify intent: is len from
userspace or always recomputed?

> +	new = kstrndup(s, len, GFP_KERNEL);
> +	if (!new)
> +		return -ENOMEM;
> +
> +	spin_lock(&dev->driver_override.lock);
> +	old = dev->driver_override.name;
> +	if (cp != s) {
> +		dev->driver_override.name = new;
> +		spin_unlock(&dev->driver_override.lock);
> +	} else {
> +		/* "\n" passed - clear override */
> +		dev->driver_override.name = NULL;
> +		spin_unlock(&dev->driver_override.lock);
> +
> +		kfree(new);
> +	}
> +	kfree(old);

Logic is correct but confusing: cp is set only if '\n' is found, so
the condition `if (cp != s)` is checking "was newline NOT at start".
Add a comment explaining the two paths (newline-at-start clears,
otherwise