Linux kernel mirror (for testing) git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git
kernel os linux

module: don't ignore sysfs_create_link() failures

The sysfs_create_link() return code is marked as __must_check, but the
module_add_driver() function tries hard to not care, by assigning the
return code to a variable. When building with 'make W=1', gcc still
warns because this variable is only assigned but not used:

drivers/base/module.c: In function 'module_add_driver':
drivers/base/module.c:36:6: warning: variable 'no_warn' set but not used [-Wunused-but-set-variable]

Rework the code to properly unwind and return the error code to the
caller. My reading of the original code was that it tries to
not fail when the links already exist, so keep ignoring -EEXIST
errors.

Fixes: e17e0f51aeea ("Driver core: show drivers in /sys/module/")
See-also: 4a7fb6363f2d ("add __must_check to device management code")
Signed-off-by: Arnd Bergmann <arnd@arndb.de>
Reviewed-by: Luis Chamberlain <mcgrof@kernel.org>
Link: https://lore.kernel.org/r/20240408080616.3911573-1-arnd@kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>

authored by

Arnd Bergmann and committed by
Greg Kroah-Hartman
85d2b0aa 0bb322be

+45 -15
+6 -3
drivers/base/base.h
··· 192 192 void devices_kset_move_last(struct device *dev); 193 193 194 194 #if defined(CONFIG_MODULES) && defined(CONFIG_SYSFS) 195 - void module_add_driver(struct module *mod, struct device_driver *drv); 195 + int module_add_driver(struct module *mod, struct device_driver *drv); 196 196 void module_remove_driver(struct device_driver *drv); 197 197 #else 198 - static inline void module_add_driver(struct module *mod, 199 - struct device_driver *drv) { } 198 + static inline int module_add_driver(struct module *mod, 199 + struct device_driver *drv) 200 + { 201 + return 0; 202 + } 200 203 static inline void module_remove_driver(struct device_driver *drv) { } 201 204 #endif 202 205
+8 -1
drivers/base/bus.c
··· 674 674 if (error) 675 675 goto out_del_list; 676 676 } 677 - module_add_driver(drv->owner, drv); 677 + error = module_add_driver(drv->owner, drv); 678 + if (error) { 679 + printk(KERN_ERR "%s: failed to create module links for %s\n", 680 + __func__, drv->name); 681 + goto out_detach; 682 + } 678 683 679 684 error = driver_create_file(drv, &driver_attr_uevent); 680 685 if (error) { ··· 704 699 705 700 return 0; 706 701 702 + out_detach: 703 + driver_detach(drv); 707 704 out_del_list: 708 705 klist_del(&priv->knode_bus); 709 706 out_unregister:
+31 -11
drivers/base/module.c
··· 30 30 mutex_unlock(&drivers_dir_mutex); 31 31 } 32 32 33 - void module_add_driver(struct module *mod, struct device_driver *drv) 33 + int module_add_driver(struct module *mod, struct device_driver *drv) 34 34 { 35 35 char *driver_name; 36 - int no_warn; 37 36 struct module_kobject *mk = NULL; 37 + int ret; 38 38 39 39 if (!drv) 40 - return; 40 + return 0; 41 41 42 42 if (mod) 43 43 mk = &mod->mkobj; ··· 56 56 } 57 57 58 58 if (!mk) 59 - return; 59 + return 0; 60 60 61 - /* Don't check return codes; these calls are idempotent */ 62 - no_warn = sysfs_create_link(&drv->p->kobj, &mk->kobj, "module"); 61 + ret = sysfs_create_link(&drv->p->kobj, &mk->kobj, "module"); 62 + if (ret) 63 + return ret; 64 + 63 65 driver_name = make_driver_name(drv); 64 - if (driver_name) { 65 - module_create_drivers_dir(mk); 66 - no_warn = sysfs_create_link(mk->drivers_dir, &drv->p->kobj, 67 - driver_name); 68 - kfree(driver_name); 66 + if (!driver_name) { 67 + ret = -ENOMEM; 68 + goto out; 69 69 } 70 + 71 + module_create_drivers_dir(mk); 72 + if (!mk->drivers_dir) { 73 + ret = -EINVAL; 74 + goto out; 75 + } 76 + 77 + ret = sysfs_create_link(mk->drivers_dir, &drv->p->kobj, driver_name); 78 + if (ret) 79 + goto out; 80 + 81 + kfree(driver_name); 82 + 83 + return 0; 84 + out: 85 + sysfs_remove_link(&drv->p->kobj, "module"); 86 + sysfs_remove_link(mk->drivers_dir, driver_name); 87 + kfree(driver_name); 88 + 89 + return ret; 70 90 } 71 91 72 92 void module_remove_driver(struct device_driver *drv)