Re: [PATCH v3] drivers/base: return bus_add_driver() errors

[Greg Kroah-Hartman]

On Tue, Oct 20, 2026 at 02:31:06PM +0800, Yemingxuan wrote:
> bus_add_driver() creates the driver's uevent attribute, adds the bus
> driver groups and the bind/unbind files after the driver has already
> been inserted into the bus and its module links have been made.  When
> any of these fail it logs the failure and still returns 0.
> 
> The driver is then registered but incomplete: a driver without
> bind/unbind cannot be bound or unbound from userspace, and a driver
> without its uevent attribute can no longer emit uevent notifications.
> driver_register() reports success, so neither the driver's init routine
> nor its caller retries, and nothing else ever creates the missing
> files.
> 
> The device side of the same driver model already treats the equivalent
> steps this way: device_add() returns failures of the equivalent sysfs
> operations to its caller, and device_del() undoes each one.
> driver_register() already calls bus_remove_driver() when the group
> registration after bus_add_driver() fails, and that branch is only
> unreachable for failures inside bus_add_driver() because of the
> unconditional successful return.
> 
> Propagate the errors so the existing unwind runs.  The new
> out_remove_module label calls module_remove_driver() before falling
> through to out_detach: bus_remove_driver() pairs every operation
> bus_add_driver() performs in reverse order, but the symlinks
> module_add_driver() creates live in the module's sysfs directory, not
> under drv->p->kobj, so the kobject teardown does not remove them.
> The attribute and the groups need no explicit removal because they live
> under drv->p->kobj, and kobject_cleanup() drops the sysfs directory for
> the caller.
> 
> The error messages are kept.  The return value of an initcall is not
> printed by the initcall machinery, so dropping them would turn a
> reported failure into a silent one.
> 
> These failures are not expected on a working system: driver_ktype
> always supplies sysfs_ops, so -EINVAL is unreachable, no in-tree bus
> defines a drv_group that collides with the built-in uevent, bind or
> unbind attributes, and -ENOMEM needs memory pressure.  The point is not
> a user-visible bug but keeping the driver model consistent with the
> device side and leaving the failure reported.
> 
> Signed-off-by: Yemingxuan <level_zero@xxxxxx>

Full name?

> ---
>  drivers/base/bus.c | 26 +
>  1 file changed, 5 insertions(+), 2 deletions(-)
> 
> --- a/drivers/base/bus.c	2026-10-20 14:29:46.248067968 +0800
> +++ b/drivers/base/bus.c	2026-10-20 14:29:46.266034395 +0800
> @@ -775,25 +775,28 @@
>  	if (error) {
>  		printk(KERN_ERR "%s: uevent attr (%s) failed\n",
>  			__func__, drv->name);
> +		goto out_remove_module;
>  	}
>  	error = driver_add_groups(drv, sp->bus->drv_groups);
>  	if (error) {
> -		/* How the hell do we get out of this pickle? Give up */
>  		printk(KERN_ERR "%s: driver_add_groups(%s) failed\n",
>  			__func__, drv->name);
> +		goto out_remove_module;

Are you sure that this is going to work properly?  How was this tested?
That comment is a good indication that no one will hit this, so why even
worry about any of this?

thanks,

greg k-h