From 9ee84466b7130360d07241baf4c95a553fc0a997 Mon Sep 17 00:00:00 2001 From: Souptick Joarder Date: Sat, 21 Apr 2018 01:15:24 +0530 Subject: [PATCH 01/23] fs: kernfs: Adding new return type vm_fault_t Use new return type vm_fault_t for page_mkwrite and fault handler. For now, this is just documenting that the function returns a VM_FAULT value rather than an errno. Once all instances are converted, vm_fault_t will become a distinct type. Reference id -> 1c8f422059ae ("mm: change return type to vm_fault_t") Signed-off-by: Souptick Joarder Reviewed-by: Matthew Wilcox Signed-off-by: Greg Kroah-Hartman --- fs/kernfs/file.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/fs/kernfs/file.c b/fs/kernfs/file.c index fd5ce883072e..2015d8c45e4a 100644 --- a/fs/kernfs/file.c +++ b/fs/kernfs/file.c @@ -348,11 +348,11 @@ static void kernfs_vma_open(struct vm_area_struct *vma) kernfs_put_active(of->kn); } -static int kernfs_vma_fault(struct vm_fault *vmf) +static vm_fault_t kernfs_vma_fault(struct vm_fault *vmf) { struct file *file = vmf->vma->vm_file; struct kernfs_open_file *of = kernfs_of(file); - int ret; + vm_fault_t ret; if (!of->vm_ops) return VM_FAULT_SIGBUS; @@ -368,11 +368,11 @@ static int kernfs_vma_fault(struct vm_fault *vmf) return ret; } -static int kernfs_vma_page_mkwrite(struct vm_fault *vmf) +static vm_fault_t kernfs_vma_page_mkwrite(struct vm_fault *vmf) { struct file *file = vmf->vma->vm_file; struct kernfs_open_file *of = kernfs_of(file); - int ret; + vm_fault_t ret; if (!of->vm_ops) return VM_FAULT_SIGBUS; From 84d0c27d6233a9ba0578b20f5a09701eb66cee42 Mon Sep 17 00:00:00 2001 From: Tetsuo Handa Date: Mon, 7 May 2018 19:10:31 +0900 Subject: [PATCH 02/23] driver core: Don't ignore class_dir_create_and_add() failure. syzbot is hitting WARN() at kernfs_add_one() [1]. This is because kernfs_create_link() is confused by previous device_add() call which continued without setting dev->kobj.parent field when get_device_parent() failed by memory allocation fault injection. Fix this by propagating the error from class_dir_create_and_add() to the calllers of get_device_parent(). [1] https://syzkaller.appspot.com/bug?id=fae0fb607989ea744526d1c082a5b8de6529116f Signed-off-by: Tetsuo Handa Reported-by: syzbot Cc: Greg Kroah-Hartman Cc: stable Signed-off-by: Greg Kroah-Hartman --- drivers/base/core.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/drivers/base/core.c b/drivers/base/core.c index b610816eb887..d680fd030316 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -1467,7 +1467,7 @@ class_dir_create_and_add(struct class *class, struct kobject *parent_kobj) dir = kzalloc(sizeof(*dir), GFP_KERNEL); if (!dir) - return NULL; + return ERR_PTR(-ENOMEM); dir->class = class; kobject_init(&dir->kobj, &class_dir_ktype); @@ -1477,7 +1477,7 @@ class_dir_create_and_add(struct class *class, struct kobject *parent_kobj) retval = kobject_add(&dir->kobj, parent_kobj, "%s", class->name); if (retval < 0) { kobject_put(&dir->kobj); - return NULL; + return ERR_PTR(retval); } return &dir->kobj; } @@ -1784,6 +1784,10 @@ int device_add(struct device *dev) parent = get_device(dev->parent); kobj = get_device_parent(dev, parent); + if (IS_ERR(kobj)) { + error = PTR_ERR(kobj); + goto parent_error; + } if (kobj) dev->kobj.parent = kobj; @@ -1882,6 +1886,7 @@ done: kobject_del(&dev->kobj); Error: cleanup_glue_dir(dev, glue_dir); +parent_error: put_device(parent); name_error: kfree(dev->p); @@ -2701,6 +2706,11 @@ int device_move(struct device *dev, struct device *new_parent, device_pm_lock(); new_parent = get_device(new_parent); new_parent_kobj = get_device_parent(dev, new_parent); + if (IS_ERR(new_parent_kobj)) { + error = PTR_ERR(new_parent_kobj); + put_device(new_parent); + goto out; + } pr_debug("device: '%s': %s: moving to '%s'\n", dev_name(dev), __func__, new_parent ? dev_name(new_parent) : ""); From eb33eb04926e40331750f538a58d93cde87afaa4 Mon Sep 17 00:00:00 2001 From: Andres Rodriguez Date: Thu, 10 May 2018 13:08:37 -0700 Subject: [PATCH 03/23] firmware: wrap FW_OPT_* into an enum This should let us associate enum kdoc to these values. While at it, kdocify the fw_opt. Signed-off-by: Andres Rodriguez Reviewed-by: Kees Cook Acked-by: Luis R. Rodriguez [mcgrof: coding style fixes, merge kdoc with enum move] Signed-off-by: Luis R. Rodriguez Signed-off-by: Greg Kroah-Hartman --- drivers/base/firmware_loader/fallback.c | 12 ++++---- drivers/base/firmware_loader/fallback.h | 6 ++-- drivers/base/firmware_loader/firmware.h | 37 +++++++++++++++++++------ drivers/base/firmware_loader/main.c | 6 ++-- 4 files changed, 42 insertions(+), 19 deletions(-) diff --git a/drivers/base/firmware_loader/fallback.c b/drivers/base/firmware_loader/fallback.c index 358354148dec..b57a7b3b4122 100644 --- a/drivers/base/firmware_loader/fallback.c +++ b/drivers/base/firmware_loader/fallback.c @@ -512,7 +512,7 @@ static const struct attribute_group *fw_dev_attr_groups[] = { static struct fw_sysfs * fw_create_instance(struct firmware *firmware, const char *fw_name, - struct device *device, unsigned int opt_flags) + struct device *device, enum fw_opt opt_flags) { struct fw_sysfs *fw_sysfs; struct device *f_dev; @@ -545,7 +545,7 @@ exit: * In charge of constructing a sysfs fallback interface for firmware loading. **/ static int fw_load_sysfs_fallback(struct fw_sysfs *fw_sysfs, - unsigned int opt_flags, long timeout) + enum fw_opt opt_flags, long timeout) { int retval = 0; struct device *f_dev = &fw_sysfs->dev; @@ -599,7 +599,7 @@ err_put_dev: static int fw_load_from_user_helper(struct firmware *firmware, const char *name, struct device *device, - unsigned int opt_flags) + enum fw_opt opt_flags) { struct fw_sysfs *fw_sysfs; long timeout; @@ -640,7 +640,7 @@ out_unlock: return ret; } -static bool fw_force_sysfs_fallback(unsigned int opt_flags) +static bool fw_force_sysfs_fallback(enum fw_opt opt_flags) { if (fw_fallback_config.force_sysfs_fallback) return true; @@ -649,7 +649,7 @@ static bool fw_force_sysfs_fallback(unsigned int opt_flags) return true; } -static bool fw_run_sysfs_fallback(unsigned int opt_flags) +static bool fw_run_sysfs_fallback(enum fw_opt opt_flags) { if (fw_fallback_config.ignore_sysfs_fallback) { pr_info_once("Ignoring firmware sysfs fallback due to sysctl knob\n"); @@ -664,7 +664,7 @@ static bool fw_run_sysfs_fallback(unsigned int opt_flags) int fw_sysfs_fallback(struct firmware *fw, const char *name, struct device *device, - unsigned int opt_flags, + enum fw_opt opt_flags, int ret) { if (!fw_run_sysfs_fallback(opt_flags)) diff --git a/drivers/base/firmware_loader/fallback.h b/drivers/base/firmware_loader/fallback.h index f8255670a663..a3b73a09db6c 100644 --- a/drivers/base/firmware_loader/fallback.h +++ b/drivers/base/firmware_loader/fallback.h @@ -5,6 +5,8 @@ #include #include +#include "firmware.h" + /** * struct firmware_fallback_config - firmware fallback configuration settings * @@ -31,7 +33,7 @@ struct firmware_fallback_config { #ifdef CONFIG_FW_LOADER_USER_HELPER int fw_sysfs_fallback(struct firmware *fw, const char *name, struct device *device, - unsigned int opt_flags, + enum fw_opt opt_flags, int ret); void kill_pending_fw_fallback_reqs(bool only_kill_custom); @@ -43,7 +45,7 @@ void unregister_sysfs_loader(void); #else /* CONFIG_FW_LOADER_USER_HELPER */ static inline int fw_sysfs_fallback(struct firmware *fw, const char *name, struct device *device, - unsigned int opt_flags, + enum fw_opt opt_flags, int ret) { /* Keep carrying over the same error */ diff --git a/drivers/base/firmware_loader/firmware.h b/drivers/base/firmware_loader/firmware.h index 64acbb1a392c..4f433b447367 100644 --- a/drivers/base/firmware_loader/firmware.h +++ b/drivers/base/firmware_loader/firmware.h @@ -2,6 +2,7 @@ #ifndef __FIRMWARE_LOADER_H #define __FIRMWARE_LOADER_H +#include #include #include #include @@ -10,13 +11,33 @@ #include -/* firmware behavior options */ -#define FW_OPT_UEVENT (1U << 0) -#define FW_OPT_NOWAIT (1U << 1) -#define FW_OPT_USERHELPER (1U << 2) -#define FW_OPT_NO_WARN (1U << 3) -#define FW_OPT_NOCACHE (1U << 4) -#define FW_OPT_NOFALLBACK (1U << 5) +/** + * enum fw_opt - options to control firmware loading behaviour + * + * @FW_OPT_UEVENT: Enables the fallback mechanism to send a kobject uevent + * when the firmware is not found. Userspace is in charge to load the + * firmware using the sysfs loading facility. + * @FW_OPT_NOWAIT: Used to describe the firmware request is asynchronous. + * @FW_OPT_USERHELPER: Enable the fallback mechanism, in case the direct + * filesystem lookup fails at finding the firmware. For details refer to + * fw_sysfs_fallback(). + * @FW_OPT_NO_WARN: Quiet, avoid printing warning messages. + * @FW_OPT_NOCACHE: Disables firmware caching. Firmware caching is used to + * cache the firmware upon suspend, so that upon resume races against the + * firmware file lookup on storage is avoided. Used for calls where the + * file may be too big, or where the driver takes charge of its own + * firmware caching mechanism. + * @FW_OPT_NOFALLBACK: Disable the fallback mechanism. Takes precedence over + * &FW_OPT_UEVENT and &FW_OPT_USERHELPER. + */ +enum fw_opt { + FW_OPT_UEVENT = BIT(0), + FW_OPT_NOWAIT = BIT(1), + FW_OPT_USERHELPER = BIT(2), + FW_OPT_NO_WARN = BIT(3), + FW_OPT_NOCACHE = BIT(4), + FW_OPT_NOFALLBACK = BIT(5), +}; enum fw_status { FW_STATUS_UNKNOWN, @@ -110,6 +131,6 @@ static inline void fw_state_done(struct fw_priv *fw_priv) } int assign_fw(struct firmware *fw, struct device *device, - unsigned int opt_flags); + enum fw_opt opt_flags); #endif /* __FIRMWARE_LOADER_H */ diff --git a/drivers/base/firmware_loader/main.c b/drivers/base/firmware_loader/main.c index eb34089e4299..9919f0e6a7cc 100644 --- a/drivers/base/firmware_loader/main.c +++ b/drivers/base/firmware_loader/main.c @@ -443,7 +443,7 @@ static int fw_add_devm_name(struct device *dev, const char *name) #endif int assign_fw(struct firmware *fw, struct device *device, - unsigned int opt_flags) + enum fw_opt opt_flags) { struct fw_priv *fw_priv = fw->priv; int ret; @@ -558,7 +558,7 @@ static void fw_abort_batch_reqs(struct firmware *fw) static int _request_firmware(const struct firmware **firmware_p, const char *name, struct device *device, void *buf, size_t size, - unsigned int opt_flags) + enum fw_opt opt_flags) { struct firmware *fw = NULL; int ret; @@ -734,7 +734,7 @@ struct firmware_work { struct device *device; void *context; void (*cont)(const struct firmware *fw, void *context); - unsigned int opt_flags; + enum fw_opt opt_flags; }; static void request_firmware_work_func(struct work_struct *work) From c35f9cbb1df8f17d398112173024a76964b5154d Mon Sep 17 00:00:00 2001 From: Andres Rodriguez Date: Thu, 10 May 2018 13:08:38 -0700 Subject: [PATCH 04/23] firmware: use () to terminate kernel-doc function names The kernel-doc spec dictates a function name ends in (). Signed-off-by: Andres Rodriguez Reviewed-by: Kees Cook Acked-by: Randy Dunlap Acked-by: Luis R. Rodriguez [mcgrof: adjust since the wide API rename is not yet merged] Signed-off-by: Luis R. Rodriguez Signed-off-by: Greg Kroah-Hartman --- drivers/base/firmware_loader/fallback.c | 8 ++++---- drivers/base/firmware_loader/main.c | 22 +++++++++++----------- 2 files changed, 15 insertions(+), 15 deletions(-) diff --git a/drivers/base/firmware_loader/fallback.c b/drivers/base/firmware_loader/fallback.c index b57a7b3b4122..529f7013616f 100644 --- a/drivers/base/firmware_loader/fallback.c +++ b/drivers/base/firmware_loader/fallback.c @@ -125,7 +125,7 @@ static ssize_t timeout_show(struct class *class, struct class_attribute *attr, } /** - * firmware_timeout_store - set number of seconds to wait for firmware + * firmware_timeout_store() - set number of seconds to wait for firmware * @class: device class pointer * @attr: device attribute pointer * @buf: buffer to scan for timeout value @@ -239,7 +239,7 @@ static int map_fw_priv_pages(struct fw_priv *fw_priv) } /** - * firmware_loading_store - set value in the 'loading' control file + * firmware_loading_store() - set value in the 'loading' control file * @dev: device pointer * @attr: device attribute pointer * @buf: buffer to scan for loading control value @@ -431,7 +431,7 @@ static int fw_realloc_pages(struct fw_sysfs *fw_sysfs, int min_size) } /** - * firmware_data_write - write method for firmware + * firmware_data_write() - write method for firmware * @filp: open sysfs file * @kobj: kobject for the device * @bin_attr: bin_attr structure @@ -537,7 +537,7 @@ exit: } /** - * fw_load_sysfs_fallback - load a firmware via the sysfs fallback mechanism + * fw_load_sysfs_fallback() - load a firmware via the sysfs fallback mechanism * @fw_sysfs: firmware sysfs information for the firmware to load * @opt_flags: flags of options, FW_OPT_* * @timeout: timeout to wait for the load diff --git a/drivers/base/firmware_loader/main.c b/drivers/base/firmware_loader/main.c index 9919f0e6a7cc..4d11efadb3a4 100644 --- a/drivers/base/firmware_loader/main.c +++ b/drivers/base/firmware_loader/main.c @@ -597,7 +597,7 @@ _request_firmware(const struct firmware **firmware_p, const char *name, } /** - * request_firmware: - send firmware request and wait for it + * request_firmware() - send firmware request and wait for it * @firmware_p: pointer to firmware image * @name: name of firmware file * @device: device for which firmware is being loaded @@ -632,7 +632,7 @@ request_firmware(const struct firmware **firmware_p, const char *name, EXPORT_SYMBOL(request_firmware); /** - * request_firmware_direct: - load firmware directly without usermode helper + * request_firmware_direct() - load firmware directly without usermode helper * @firmware_p: pointer to firmware image * @name: name of firmware file * @device: device for which firmware is being loaded @@ -657,7 +657,7 @@ int request_firmware_direct(const struct firmware **firmware_p, EXPORT_SYMBOL_GPL(request_firmware_direct); /** - * firmware_request_cache: - cache firmware for suspend so resume can use it + * firmware_request_cache() - cache firmware for suspend so resume can use it * @name: name of firmware file * @device: device for which firmware should be cached for * @@ -681,7 +681,7 @@ int firmware_request_cache(struct device *device, const char *name) EXPORT_SYMBOL_GPL(firmware_request_cache); /** - * request_firmware_into_buf - load firmware into a previously allocated buffer + * request_firmware_into_buf() - load firmware into a previously allocated buffer * @firmware_p: pointer to firmware image * @name: name of firmware file * @device: device for which firmware is being loaded and DMA region allocated @@ -713,7 +713,7 @@ request_firmware_into_buf(const struct firmware **firmware_p, const char *name, EXPORT_SYMBOL(request_firmware_into_buf); /** - * release_firmware: - release the resource associated with a firmware image + * release_firmware() - release the resource associated with a firmware image * @fw: firmware resource to release **/ void release_firmware(const struct firmware *fw) @@ -755,7 +755,7 @@ static void request_firmware_work_func(struct work_struct *work) } /** - * request_firmware_nowait - asynchronous version of request_firmware + * request_firmware_nowait() - asynchronous version of request_firmware * @module: module requesting the firmware * @uevent: sends uevent to copy the firmware image if this flag * is non-zero else the firmware copy must be done manually. @@ -824,7 +824,7 @@ EXPORT_SYMBOL(request_firmware_nowait); static ASYNC_DOMAIN_EXCLUSIVE(fw_cache_domain); /** - * cache_firmware - cache one firmware image in kernel memory space + * cache_firmware() - cache one firmware image in kernel memory space * @fw_name: the firmware image name * * Cache firmware in kernel memory so that drivers can use it when @@ -866,7 +866,7 @@ static struct fw_priv *lookup_fw_priv(const char *fw_name) } /** - * uncache_firmware - remove one cached firmware image + * uncache_firmware() - remove one cached firmware image * @fw_name: the firmware image name * * Uncache one firmware image which has been cached successfully @@ -1042,7 +1042,7 @@ static void __device_uncache_fw_images(void) } /** - * device_cache_fw_images - cache devices' firmware + * device_cache_fw_images() - cache devices' firmware * * If one device called request_firmware or its nowait version * successfully before, the firmware names are recored into the @@ -1075,7 +1075,7 @@ static void device_cache_fw_images(void) } /** - * device_uncache_fw_images - uncache devices' firmware + * device_uncache_fw_images() - uncache devices' firmware * * uncache all firmwares which have been cached successfully * by device_uncache_fw_images earlier @@ -1092,7 +1092,7 @@ static void device_uncache_fw_images_work(struct work_struct *work) } /** - * device_uncache_fw_images_delay - uncache devices firmwares + * device_uncache_fw_images_delay() - uncache devices firmwares * @delay: number of milliseconds to delay uncache device firmwares * * uncache all devices's firmwares which has been cached successfully From cf1cde7cd6e42aa65aa7a80e4980afe6d1a1330e Mon Sep 17 00:00:00 2001 From: Andres Rodriguez Date: Thu, 10 May 2018 13:08:39 -0700 Subject: [PATCH 05/23] firmware: rename fw_sysfs_fallback to firmware_fallback_sysfs() This is done since this call is now exposed through kernel-doc, and since this also paves the way for different future types of fallback mechanims. Signed-off-by: Andres Rodriguez Reviewed-by: Kees Cook Acked-by: Luis R. Rodriguez [mcgrof: small coding style changes] Signed-off-by: Luis R. Rodriguez Signed-off-by: Greg Kroah-Hartman --- drivers/base/firmware_loader/fallback.c | 8 ++++---- drivers/base/firmware_loader/fallback.h | 16 ++++++++-------- drivers/base/firmware_loader/firmware.h | 2 +- drivers/base/firmware_loader/main.c | 2 +- 4 files changed, 14 insertions(+), 14 deletions(-) diff --git a/drivers/base/firmware_loader/fallback.c b/drivers/base/firmware_loader/fallback.c index 529f7013616f..3db9e0f225ac 100644 --- a/drivers/base/firmware_loader/fallback.c +++ b/drivers/base/firmware_loader/fallback.c @@ -662,10 +662,10 @@ static bool fw_run_sysfs_fallback(enum fw_opt opt_flags) return fw_force_sysfs_fallback(opt_flags); } -int fw_sysfs_fallback(struct firmware *fw, const char *name, - struct device *device, - enum fw_opt opt_flags, - int ret) +int firmware_fallback_sysfs(struct firmware *fw, const char *name, + struct device *device, + enum fw_opt opt_flags, + int ret) { if (!fw_run_sysfs_fallback(opt_flags)) return ret; diff --git a/drivers/base/firmware_loader/fallback.h b/drivers/base/firmware_loader/fallback.h index a3b73a09db6c..21063503e4ea 100644 --- a/drivers/base/firmware_loader/fallback.h +++ b/drivers/base/firmware_loader/fallback.h @@ -31,10 +31,10 @@ struct firmware_fallback_config { }; #ifdef CONFIG_FW_LOADER_USER_HELPER -int fw_sysfs_fallback(struct firmware *fw, const char *name, - struct device *device, - enum fw_opt opt_flags, - int ret); +int firmware_fallback_sysfs(struct firmware *fw, const char *name, + struct device *device, + enum fw_opt opt_flags, + int ret); void kill_pending_fw_fallback_reqs(bool only_kill_custom); void fw_fallback_set_cache_timeout(void); @@ -43,10 +43,10 @@ void fw_fallback_set_default_timeout(void); int register_sysfs_loader(void); void unregister_sysfs_loader(void); #else /* CONFIG_FW_LOADER_USER_HELPER */ -static inline int fw_sysfs_fallback(struct firmware *fw, const char *name, - struct device *device, - enum fw_opt opt_flags, - int ret) +static inline int firmware_fallback_sysfs(struct firmware *fw, const char *name, + struct device *device, + enum fw_opt opt_flags, + int ret) { /* Keep carrying over the same error */ return ret; diff --git a/drivers/base/firmware_loader/firmware.h b/drivers/base/firmware_loader/firmware.h index 4f433b447367..4c1395f8e7ed 100644 --- a/drivers/base/firmware_loader/firmware.h +++ b/drivers/base/firmware_loader/firmware.h @@ -20,7 +20,7 @@ * @FW_OPT_NOWAIT: Used to describe the firmware request is asynchronous. * @FW_OPT_USERHELPER: Enable the fallback mechanism, in case the direct * filesystem lookup fails at finding the firmware. For details refer to - * fw_sysfs_fallback(). + * firmware_fallback_sysfs(). * @FW_OPT_NO_WARN: Quiet, avoid printing warning messages. * @FW_OPT_NOCACHE: Disables firmware caching. Firmware caching is used to * cache the firmware upon suspend, so that upon resume races against the diff --git a/drivers/base/firmware_loader/main.c b/drivers/base/firmware_loader/main.c index 4d11efadb3a4..abdc4e4d55f1 100644 --- a/drivers/base/firmware_loader/main.c +++ b/drivers/base/firmware_loader/main.c @@ -581,7 +581,7 @@ _request_firmware(const struct firmware **firmware_p, const char *name, dev_warn(device, "Direct firmware load for %s failed with error %d\n", name, ret); - ret = fw_sysfs_fallback(fw, name, device, opt_flags, ret); + ret = firmware_fallback_sysfs(fw, name, device, opt_flags, ret); } else ret = assign_fw(fw, device, opt_flags); From 84b5c4fec73353a8946fe3f2d43e407d7a53a050 Mon Sep 17 00:00:00 2001 From: "Luis R. Rodriguez" Date: Thu, 10 May 2018 13:08:40 -0700 Subject: [PATCH 06/23] firmware_loader: document firmware_sysfs_fallback() This also sets the expecations for future fallback interfaces, even if they are not exported. Reviewed-by: Kees Cook Signed-off-by: Luis R. Rodriguez Signed-off-by: Greg Kroah-Hartman --- drivers/base/firmware_loader/fallback.c | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/drivers/base/firmware_loader/fallback.c b/drivers/base/firmware_loader/fallback.c index 3db9e0f225ac..9169e7b9800c 100644 --- a/drivers/base/firmware_loader/fallback.c +++ b/drivers/base/firmware_loader/fallback.c @@ -662,6 +662,26 @@ static bool fw_run_sysfs_fallback(enum fw_opt opt_flags) return fw_force_sysfs_fallback(opt_flags); } +/** + * firmware_fallback_sysfs() - use the fallback mechanism to find firmware + * @fw: pointer to firmware image + * @name: name of firmware file to look for + * @device: device for which firmware is being loaded + * @opt_flags: options to control firmware loading behaviour + * @ret: return value from direct lookup which triggered the fallback mechanism + * + * This function is called if direct lookup for the firmware failed, it enables + * a fallback mechanism through userspace by exposing a sysfs loading + * interface. Userspace is in charge of loading the firmware through the syfs + * loading interface. This syfs fallback mechanism may be disabled completely + * on a system by setting the proc sysctl value ignore_sysfs_fallback to true. + * If this false we check if the internal API caller set the @FW_OPT_NOFALLBACK + * flag, if so it would also disable the fallback mechanism. A system may want + * to enfoce the sysfs fallback mechanism at all times, it can do this by + * setting ignore_sysfs_fallback to false and force_sysfs_fallback to true. + * Enabling force_sysfs_fallback is functionally equivalent to build a kernel + * with CONFIG_FW_LOADER_USER_HELPER_FALLBACK. + **/ int firmware_fallback_sysfs(struct firmware *fw, const char *name, struct device *device, enum fw_opt opt_flags, From 02c399306826412562ec68712eada97e8e355f35 Mon Sep 17 00:00:00 2001 From: "Luis R. Rodriguez" Date: Thu, 10 May 2018 13:08:41 -0700 Subject: [PATCH 07/23] firmware_loader: enhance Kconfig documentation over FW_LOADER If you try to read FW_LOADER today it speaks of old riddles and unless you have been following development closely you will lose track of what is what. Even the documentation for PREVENT_FIRMWARE_BUILD is a bit fuzzy and how it fits into this big picture. Give the FW_LOADER kconfig documentation some love with more up to date developments and recommendations. While at it, wrap the FW_LOADER code into its own menu to compartmentalize and make it clearer which components really are part of the FW_LOADER. This should also make it easier to later move these kconfig entries into the firmware_loader/ directory later. This also now recommends using firmwared [0] for folks left needing a uevent handler in userspace for the sysfs firmware fallback mechanis given udev's uevent firmware mechanism was ripped out a while ago. [0] https://github.com/teg/firmwared Reviewed-by: Kees Cook Signed-off-by: Luis R. Rodriguez Signed-off-by: Greg Kroah-Hartman --- drivers/base/Kconfig | 165 ++++++++++++++++++++++++++++++++++--------- 1 file changed, 131 insertions(+), 34 deletions(-) diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig index 29b0eb452b3a..db2bbe483927 100644 --- a/drivers/base/Kconfig +++ b/drivers/base/Kconfig @@ -70,39 +70,64 @@ config STANDALONE If unsure, say Y. config PREVENT_FIRMWARE_BUILD - bool "Prevent firmware from being built" + bool "Disable drivers features which enable custom firmware building" default y help - Say yes to avoid building firmware. Firmware is usually shipped - with the driver and only when updating the firmware should a - rebuild be made. - If unsure, say Y here. + Say yes to disable driver features which enable building a custom + driver firmware at kernel build time. These drivers do not use the + kernel firmware API to load firmware (CONFIG_FW_LOADER), instead they + use their own custom loading mechanism. The required firmware is + usually shipped with the driver, building the driver firmware + should only be needed if you have an updated firmware source. + + Firmware should not be being built as part of kernel, these days + you should always prevent this and say Y here. There are only two + old drivers which enable building of its firmware at kernel build + time: + + o CONFIG_WANXL through CONFIG_WANXL_BUILD_FIRMWARE + o CONFIG_SCSI_AIC79XX through CONFIG_AIC79XX_BUILD_FIRMWARE + +menu "Firmware loader" config FW_LOADER - tristate "Userspace firmware loading support" if EXPERT + tristate "Firmware loading facility" if EXPERT default y ---help--- - This option is provided for the case where none of the in-tree modules - require userspace firmware loading support, but a module built - out-of-tree does. + This enables the firmware loading facility in the kernel. The kernel + will first look for built-in firmware, if it has any. Next, it will + look for the requested firmware in a series of filesystem paths: + + o firmware_class path module parameter or kernel boot param + o /lib/firmware/updates/UTS_RELEASE + o /lib/firmware/updates + o /lib/firmware/UTS_RELEASE + o /lib/firmware + + Enabling this feature only increases your kernel image by about + 828 bytes, enable this option unless you are certain you don't + need firmware. + + You typically want this built-in (=y) but you can also enable this + as a module, in which case the firmware_class module will be built. + You also want to be sure to enable this built-in if you are going to + enable built-in firmware (CONFIG_EXTRA_FIRMWARE). + +if FW_LOADER config EXTRA_FIRMWARE - string "External firmware blobs to build into the kernel binary" - depends on FW_LOADER + string "Build named firmware blobs into the kernel binary" help - Various drivers in the kernel source tree may require firmware, - which is generally available in your distribution's linux-firmware - package. + Device drivers which require firmware can typically deal with + having the kernel load firmware from the various supported + /lib/firmware/ paths. This option enables you to build into the + kernel firmware files. Built-in firmware searches are preceded + over firmware lookups using your filesystem over the supported + /lib/firmware paths documented on CONFIG_FW_LOADER. - The linux-firmware package should install firmware into - /lib/firmware/ on your system, so they can be loaded by userspace - helpers on request. - - This option allows firmware to be built into the kernel for the case - where the user either cannot or doesn't want to provide it from - userspace at runtime (for example, when the firmware in question is - required for accessing the boot device, and the user doesn't want to - use an initrd). + This may be useful for testing or if the firmware is required early on + in boot and cannot rely on the firmware being placed in an initrd or + initramfs. This option is a string and takes the (space-separated) names of the firmware files -- the same names that appear in MODULE_FIRMWARE() @@ -113,7 +138,7 @@ config EXTRA_FIRMWARE For example, you might set CONFIG_EXTRA_FIRMWARE="usb8388.bin", copy the usb8388.bin file into /lib/firmware, and build the kernel. Then any request_firmware("usb8388.bin") will be satisfied internally - without needing to call out to userspace. + inside the kernel without ever looking at your filesystem at runtime. WARNING: If you include additional firmware files into your binary kernel image that are not available under the terms of the GPL, @@ -130,22 +155,94 @@ config EXTRA_FIRMWARE_DIR looks for the firmware files listed in the EXTRA_FIRMWARE option. config FW_LOADER_USER_HELPER - bool + bool "Enable the firmware sysfs fallback mechanism" + help + This option enables a sysfs loading facility to enable firmware + loading to the kernel through userspace as a fallback mechanism + if and only if the kernel's direct filesystem lookup for the + firmware failed using the different /lib/firmware/ paths, or the + path specified in the firmware_class path module parameter, or the + firmware_class path kernel boot parameter if the firmware_class is + built-in. For details on how to work with the sysfs fallback mechanism + refer to Documentation/driver-api/firmware/fallback-mechanisms.rst. + + The direct filesystem lookup for firmware is always used first now. + + If the kernel's direct filesystem lookup for firmware fails to find + the requested firmware a sysfs fallback loading facility is made + available and userspace is informed about this through uevents. + The uevent can be suppressed if the driver explicitly requested it, + this is known as the driver using the custom fallback mechanism. + If the custom fallback mechanism is used userspace must always + acknowledge failure to find firmware as the timeout for the fallback + mechanism is disabled, and failed requests will linger forever. + + This used to be the default firmware loading facility, and udev used + to listen for uvents to load firmware for the kernel. The firmware + loading facility functionality in udev has been removed, as such it + can no longer be relied upon as a fallback mechanism. Linux no longer + relies on or uses a fallback mechanism in userspace. If you need to + rely on one refer to the permissively licensed firmwared: + + https://github.com/teg/firmwared + + Since this was the default firmware loading facility at one point, + old userspace may exist which relies upon it, and as such this + mechanism can never be removed from the kernel. + + You should only enable this functionality if you are certain you + require a fallback mechanism and have a userspace mechanism ready to + load firmware in case it is not found. One main reason for this may + be if you have drivers which require firmware built-in and for + whatever reason cannot place the required firmware in initramfs. + Another reason kernels may have this feature enabled is to support a + driver which explicitly relies on this fallback mechanism. Only two + drivers need this today: + + o CONFIG_LEDS_LP55XX_COMMON + o CONFIG_DELL_RBU + + Outside of supporting the above drivers, another reason for needing + this may be that your firmware resides outside of the paths the kernel + looks for and cannot possibly be specified using the firmware_class + path module parameter or kernel firmware_class path boot parameter + if firmware_class is built-in. + + A modern use case may be to temporarily mount a custom partition + during provisioning which is only accessible to userspace, and then + to use it to look for and fetch the required firmware. Such type of + driver functionality may not even ever be desirable upstream by + vendors, and as such is only required to be supported as an interface + for provisioning. Since udev's firmware loading facility has been + removed you can use firmwared or a fork of it to customize how you + want to load firmware based on uevents issued. + + Enabling this option will increase your kernel image size by about + 13436 bytes. + + If you are unsure about this, say N here, unless you are Linux + distribution and need to support the above two drivers, or you are + certain you need to support some really custom firmware loading + facility in userspace. config FW_LOADER_USER_HELPER_FALLBACK - bool "Fallback user-helper invocation for firmware loading" - depends on FW_LOADER - select FW_LOADER_USER_HELPER + bool "Force the firmware sysfs fallback mechanism when possible" + depends on FW_LOADER_USER_HELPER help - This option enables / disables the invocation of user-helper - (e.g. udev) for loading firmware files as a fallback after the - direct file loading in kernel fails. The user-mode helper is - no longer required unless you have a special firmware file that - resides in a non-standard path. Moreover, the udev support has - been deprecated upstream. + Enabling this option forces a sysfs userspace fallback mechanism + to be used for all firmware requests which explicitly do not disable a + a fallback mechanism. Firmware calls which do prohibit a fallback + mechanism is request_firmware_direct(). This option is kept for + backward compatibility purposes given this precise mechanism can also + be enabled by setting the proc sysctl value to true: + + /proc/sys/kernel/firmware_config/force_sysfs_fallback If you are unsure about this, say N here. +endif # FW_LOADER +endmenu + config WANT_DEV_COREDUMP bool help From 367d09824193e5a9aea98490ae0506cec8abe9c4 Mon Sep 17 00:00:00 2001 From: "Luis R. Rodriguez" Date: Thu, 10 May 2018 13:08:42 -0700 Subject: [PATCH 08/23] firmware_loader: replace ---help--- with help As per checkpatch using help is preferred over ---help---. Signed-off-by: Luis R. Rodriguez Signed-off-by: Greg Kroah-Hartman --- drivers/base/Kconfig | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig index db2bbe483927..0c38df32c7fe 100644 --- a/drivers/base/Kconfig +++ b/drivers/base/Kconfig @@ -93,7 +93,7 @@ menu "Firmware loader" config FW_LOADER tristate "Firmware loading facility" if EXPERT default y - ---help--- + help This enables the firmware loading facility in the kernel. The kernel will first look for built-in firmware, if it has any. Next, it will look for the requested firmware in a series of filesystem paths: From 06bfd3c8ab1dbf0031022d056a90ace682f6a94c Mon Sep 17 00:00:00 2001 From: "Luis R. Rodriguez" Date: Thu, 10 May 2018 13:08:43 -0700 Subject: [PATCH 09/23] firmware_loader: move kconfig FW_LOADER entries to its own file This will make it easier to track and easier to understand what components and features are part of the FW_LOADER. There are some components related to firmware which have *nothing* to do with the FW_LOADER, souch as PREVENT_FIRMWARE_BUILD. Reviewed-by: Kees Cook Signed-off-by: Luis R. Rodriguez Signed-off-by: Greg Kroah-Hartman --- drivers/base/Kconfig | 155 +-------------------------- drivers/base/firmware_loader/Kconfig | 154 ++++++++++++++++++++++++++ 2 files changed, 155 insertions(+), 154 deletions(-) create mode 100644 drivers/base/firmware_loader/Kconfig diff --git a/drivers/base/Kconfig b/drivers/base/Kconfig index 0c38df32c7fe..3e63a900b330 100644 --- a/drivers/base/Kconfig +++ b/drivers/base/Kconfig @@ -88,160 +88,7 @@ config PREVENT_FIRMWARE_BUILD o CONFIG_WANXL through CONFIG_WANXL_BUILD_FIRMWARE o CONFIG_SCSI_AIC79XX through CONFIG_AIC79XX_BUILD_FIRMWARE -menu "Firmware loader" - -config FW_LOADER - tristate "Firmware loading facility" if EXPERT - default y - help - This enables the firmware loading facility in the kernel. The kernel - will first look for built-in firmware, if it has any. Next, it will - look for the requested firmware in a series of filesystem paths: - - o firmware_class path module parameter or kernel boot param - o /lib/firmware/updates/UTS_RELEASE - o /lib/firmware/updates - o /lib/firmware/UTS_RELEASE - o /lib/firmware - - Enabling this feature only increases your kernel image by about - 828 bytes, enable this option unless you are certain you don't - need firmware. - - You typically want this built-in (=y) but you can also enable this - as a module, in which case the firmware_class module will be built. - You also want to be sure to enable this built-in if you are going to - enable built-in firmware (CONFIG_EXTRA_FIRMWARE). - -if FW_LOADER - -config EXTRA_FIRMWARE - string "Build named firmware blobs into the kernel binary" - help - Device drivers which require firmware can typically deal with - having the kernel load firmware from the various supported - /lib/firmware/ paths. This option enables you to build into the - kernel firmware files. Built-in firmware searches are preceded - over firmware lookups using your filesystem over the supported - /lib/firmware paths documented on CONFIG_FW_LOADER. - - This may be useful for testing or if the firmware is required early on - in boot and cannot rely on the firmware being placed in an initrd or - initramfs. - - This option is a string and takes the (space-separated) names of the - firmware files -- the same names that appear in MODULE_FIRMWARE() - and request_firmware() in the source. These files should exist under - the directory specified by the EXTRA_FIRMWARE_DIR option, which is - /lib/firmware by default. - - For example, you might set CONFIG_EXTRA_FIRMWARE="usb8388.bin", copy - the usb8388.bin file into /lib/firmware, and build the kernel. Then - any request_firmware("usb8388.bin") will be satisfied internally - inside the kernel without ever looking at your filesystem at runtime. - - WARNING: If you include additional firmware files into your binary - kernel image that are not available under the terms of the GPL, - then it may be a violation of the GPL to distribute the resulting - image since it combines both GPL and non-GPL work. You should - consult a lawyer of your own before distributing such an image. - -config EXTRA_FIRMWARE_DIR - string "Firmware blobs root directory" - depends on EXTRA_FIRMWARE != "" - default "/lib/firmware" - help - This option controls the directory in which the kernel build system - looks for the firmware files listed in the EXTRA_FIRMWARE option. - -config FW_LOADER_USER_HELPER - bool "Enable the firmware sysfs fallback mechanism" - help - This option enables a sysfs loading facility to enable firmware - loading to the kernel through userspace as a fallback mechanism - if and only if the kernel's direct filesystem lookup for the - firmware failed using the different /lib/firmware/ paths, or the - path specified in the firmware_class path module parameter, or the - firmware_class path kernel boot parameter if the firmware_class is - built-in. For details on how to work with the sysfs fallback mechanism - refer to Documentation/driver-api/firmware/fallback-mechanisms.rst. - - The direct filesystem lookup for firmware is always used first now. - - If the kernel's direct filesystem lookup for firmware fails to find - the requested firmware a sysfs fallback loading facility is made - available and userspace is informed about this through uevents. - The uevent can be suppressed if the driver explicitly requested it, - this is known as the driver using the custom fallback mechanism. - If the custom fallback mechanism is used userspace must always - acknowledge failure to find firmware as the timeout for the fallback - mechanism is disabled, and failed requests will linger forever. - - This used to be the default firmware loading facility, and udev used - to listen for uvents to load firmware for the kernel. The firmware - loading facility functionality in udev has been removed, as such it - can no longer be relied upon as a fallback mechanism. Linux no longer - relies on or uses a fallback mechanism in userspace. If you need to - rely on one refer to the permissively licensed firmwared: - - https://github.com/teg/firmwared - - Since this was the default firmware loading facility at one point, - old userspace may exist which relies upon it, and as such this - mechanism can never be removed from the kernel. - - You should only enable this functionality if you are certain you - require a fallback mechanism and have a userspace mechanism ready to - load firmware in case it is not found. One main reason for this may - be if you have drivers which require firmware built-in and for - whatever reason cannot place the required firmware in initramfs. - Another reason kernels may have this feature enabled is to support a - driver which explicitly relies on this fallback mechanism. Only two - drivers need this today: - - o CONFIG_LEDS_LP55XX_COMMON - o CONFIG_DELL_RBU - - Outside of supporting the above drivers, another reason for needing - this may be that your firmware resides outside of the paths the kernel - looks for and cannot possibly be specified using the firmware_class - path module parameter or kernel firmware_class path boot parameter - if firmware_class is built-in. - - A modern use case may be to temporarily mount a custom partition - during provisioning which is only accessible to userspace, and then - to use it to look for and fetch the required firmware. Such type of - driver functionality may not even ever be desirable upstream by - vendors, and as such is only required to be supported as an interface - for provisioning. Since udev's firmware loading facility has been - removed you can use firmwared or a fork of it to customize how you - want to load firmware based on uevents issued. - - Enabling this option will increase your kernel image size by about - 13436 bytes. - - If you are unsure about this, say N here, unless you are Linux - distribution and need to support the above two drivers, or you are - certain you need to support some really custom firmware loading - facility in userspace. - -config FW_LOADER_USER_HELPER_FALLBACK - bool "Force the firmware sysfs fallback mechanism when possible" - depends on FW_LOADER_USER_HELPER - help - Enabling this option forces a sysfs userspace fallback mechanism - to be used for all firmware requests which explicitly do not disable a - a fallback mechanism. Firmware calls which do prohibit a fallback - mechanism is request_firmware_direct(). This option is kept for - backward compatibility purposes given this precise mechanism can also - be enabled by setting the proc sysctl value to true: - - /proc/sys/kernel/firmware_config/force_sysfs_fallback - - If you are unsure about this, say N here. - -endif # FW_LOADER -endmenu +source "drivers/base/firmware_loader/Kconfig" config WANT_DEV_COREDUMP bool diff --git a/drivers/base/firmware_loader/Kconfig b/drivers/base/firmware_loader/Kconfig new file mode 100644 index 000000000000..eb15d976a9ea --- /dev/null +++ b/drivers/base/firmware_loader/Kconfig @@ -0,0 +1,154 @@ +menu "Firmware loader" + +config FW_LOADER + tristate "Firmware loading facility" if EXPERT + default y + help + This enables the firmware loading facility in the kernel. The kernel + will first look for built-in firmware, if it has any. Next, it will + look for the requested firmware in a series of filesystem paths: + + o firmware_class path module parameter or kernel boot param + o /lib/firmware/updates/UTS_RELEASE + o /lib/firmware/updates + o /lib/firmware/UTS_RELEASE + o /lib/firmware + + Enabling this feature only increases your kernel image by about + 828 bytes, enable this option unless you are certain you don't + need firmware. + + You typically want this built-in (=y) but you can also enable this + as a module, in which case the firmware_class module will be built. + You also want to be sure to enable this built-in if you are going to + enable built-in firmware (CONFIG_EXTRA_FIRMWARE). + +if FW_LOADER + +config EXTRA_FIRMWARE + string "Build named firmware blobs into the kernel binary" + help + Device drivers which require firmware can typically deal with + having the kernel load firmware from the various supported + /lib/firmware/ paths. This option enables you to build into the + kernel firmware files. Built-in firmware searches are preceded + over firmware lookups using your filesystem over the supported + /lib/firmware paths documented on CONFIG_FW_LOADER. + + This may be useful for testing or if the firmware is required early on + in boot and cannot rely on the firmware being placed in an initrd or + initramfs. + + This option is a string and takes the (space-separated) names of the + firmware files -- the same names that appear in MODULE_FIRMWARE() + and request_firmware() in the source. These files should exist under + the directory specified by the EXTRA_FIRMWARE_DIR option, which is + /lib/firmware by default. + + For example, you might set CONFIG_EXTRA_FIRMWARE="usb8388.bin", copy + the usb8388.bin file into /lib/firmware, and build the kernel. Then + any request_firmware("usb8388.bin") will be satisfied internally + inside the kernel without ever looking at your filesystem at runtime. + + WARNING: If you include additional firmware files into your binary + kernel image that are not available under the terms of the GPL, + then it may be a violation of the GPL to distribute the resulting + image since it combines both GPL and non-GPL work. You should + consult a lawyer of your own before distributing such an image. + +config EXTRA_FIRMWARE_DIR + string "Firmware blobs root directory" + depends on EXTRA_FIRMWARE != "" + default "/lib/firmware" + help + This option controls the directory in which the kernel build system + looks for the firmware files listed in the EXTRA_FIRMWARE option. + +config FW_LOADER_USER_HELPER + bool "Enable the firmware sysfs fallback mechanism" + help + This option enables a sysfs loading facility to enable firmware + loading to the kernel through userspace as a fallback mechanism + if and only if the kernel's direct filesystem lookup for the + firmware failed using the different /lib/firmware/ paths, or the + path specified in the firmware_class path module parameter, or the + firmware_class path kernel boot parameter if the firmware_class is + built-in. For details on how to work with the sysfs fallback mechanism + refer to Documentation/driver-api/firmware/fallback-mechanisms.rst. + + The direct filesystem lookup for firmware is always used first now. + + If the kernel's direct filesystem lookup for firmware fails to find + the requested firmware a sysfs fallback loading facility is made + available and userspace is informed about this through uevents. + The uevent can be suppressed if the driver explicitly requested it, + this is known as the driver using the custom fallback mechanism. + If the custom fallback mechanism is used userspace must always + acknowledge failure to find firmware as the timeout for the fallback + mechanism is disabled, and failed requests will linger forever. + + This used to be the default firmware loading facility, and udev used + to listen for uvents to load firmware for the kernel. The firmware + loading facility functionality in udev has been removed, as such it + can no longer be relied upon as a fallback mechanism. Linux no longer + relies on or uses a fallback mechanism in userspace. If you need to + rely on one refer to the permissively licensed firmwared: + + https://github.com/teg/firmwared + + Since this was the default firmware loading facility at one point, + old userspace may exist which relies upon it, and as such this + mechanism can never be removed from the kernel. + + You should only enable this functionality if you are certain you + require a fallback mechanism and have a userspace mechanism ready to + load firmware in case it is not found. One main reason for this may + be if you have drivers which require firmware built-in and for + whatever reason cannot place the required firmware in initramfs. + Another reason kernels may have this feature enabled is to support a + driver which explicitly relies on this fallback mechanism. Only two + drivers need this today: + + o CONFIG_LEDS_LP55XX_COMMON + o CONFIG_DELL_RBU + + Outside of supporting the above drivers, another reason for needing + this may be that your firmware resides outside of the paths the kernel + looks for and cannot possibly be specified using the firmware_class + path module parameter or kernel firmware_class path boot parameter + if firmware_class is built-in. + + A modern use case may be to temporarily mount a custom partition + during provisioning which is only accessible to userspace, and then + to use it to look for and fetch the required firmware. Such type of + driver functionality may not even ever be desirable upstream by + vendors, and as such is only required to be supported as an interface + for provisioning. Since udev's firmware loading facility has been + removed you can use firmwared or a fork of it to customize how you + want to load firmware based on uevents issued. + + Enabling this option will increase your kernel image size by about + 13436 bytes. + + If you are unsure about this, say N here, unless you are Linux + distribution and need to support the above two drivers, or you are + certain you need to support some really custom firmware loading + facility in userspace. + +config FW_LOADER_USER_HELPER_FALLBACK + bool "Force the firmware sysfs fallback mechanism when possible" + depends on FW_LOADER_USER_HELPER + help + Enabling this option forces a sysfs userspace fallback mechanism + to be used for all firmware requests which explicitly do not disable a + a fallback mechanism. Firmware calls which do prohibit a fallback + mechanism is request_firmware_direct(). This option is kept for + backward compatibility purposes given this precise mechanism can also + be enabled by setting the proc sysctl value to true: + + /proc/sys/kernel/firmware_config/force_sysfs_fallback + + If you are unsure about this, say N here. + +endif # FW_LOADER +endmenu From 27d5d7dc9aafd6db3d7aeb49cdbfe578fc1b8663 Mon Sep 17 00:00:00 2001 From: "Luis R. Rodriguez" Date: Thu, 10 May 2018 13:08:44 -0700 Subject: [PATCH 10/23] firmware_loader: make firmware_fallback_sysfs() print more useful If we resort to using the sysfs fallback mechanism we don't print the filename. This can be deceiving given we could have a series of callers intertwined and it'd be unclear exactly for what firmware this was meant for. Additionally, although we don't currently use FW_OPT_NO_WARN when dealing with the fallback mechanism, we will soon, so just respect its use consistently. And even if you *don't* want to print always on failure, you may want to print when debugging so enable dynamic debug print when FW_OPT_NO_WARN is used. Reviewed-by: Kees Cook Signed-off-by: Luis R. Rodriguez Signed-off-by: Greg Kroah-Hartman --- drivers/base/firmware_loader/fallback.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/drivers/base/firmware_loader/fallback.c b/drivers/base/firmware_loader/fallback.c index 9169e7b9800c..b676a99c469c 100644 --- a/drivers/base/firmware_loader/fallback.c +++ b/drivers/base/firmware_loader/fallback.c @@ -690,6 +690,11 @@ int firmware_fallback_sysfs(struct firmware *fw, const char *name, if (!fw_run_sysfs_fallback(opt_flags)) return ret; - dev_warn(device, "Falling back to user helper\n"); + if (!(opt_flags & FW_OPT_NO_WARN)) + dev_warn(device, "Falling back to syfs fallback for: %s\n", + name); + else + dev_dbg(device, "Falling back to sysfs fallback for: %s\n", + name); return fw_load_from_user_helper(fw, name, device, opt_flags); } From 7dcc01343e483efda0882456f8361f061a5f416d Mon Sep 17 00:00:00 2001 From: Andres Rodriguez Date: Thu, 10 May 2018 13:08:45 -0700 Subject: [PATCH 11/23] firmware: add firmware_request_nowarn() - load firmware without warnings Currently the firmware loader only exposes one silent path for querying optional firmware, and that is firmware_request_direct(). This function also disables the sysfs fallback mechanism, which might not always be the desired behaviour [0]. This patch introduces a variations of request_firmware() that enable the caller to disable the undesired warning messages but enables the sysfs fallback mechanism. This is equivalent to adding FW_OPT_NO_WARN to the old behaviour. [0]: https://git.kernel.org/linus/c0cc00f250e1 Signed-off-by: Andres Rodriguez Reviewed-by: Kees Cook Acked-by: Luis R. Rodriguez [mcgrof: used the old API calls as the full rename is not done yet, and add the caller for when FW_LOADER is disabled, enhance documentation ] Signed-off-by: Luis R. Rodriguez Signed-off-by: Greg Kroah-Hartman --- .../driver-api/firmware/request_firmware.rst | 5 ++++ drivers/base/firmware_loader/main.c | 27 +++++++++++++++++++ include/linux/firmware.h | 10 +++++++ 3 files changed, 42 insertions(+) diff --git a/Documentation/driver-api/firmware/request_firmware.rst b/Documentation/driver-api/firmware/request_firmware.rst index d5ec95a7195b..f62bdcbfed5b 100644 --- a/Documentation/driver-api/firmware/request_firmware.rst +++ b/Documentation/driver-api/firmware/request_firmware.rst @@ -20,6 +20,11 @@ request_firmware .. kernel-doc:: drivers/base/firmware_loader/main.c :functions: request_firmware +firmware_request_nowarn +----------------------- +.. kernel-doc:: drivers/base/firmware_loader/main.c + :functions: firmware_request_nowarn + request_firmware_direct ----------------------- .. kernel-doc:: drivers/base/firmware_loader/main.c diff --git a/drivers/base/firmware_loader/main.c b/drivers/base/firmware_loader/main.c index abdc4e4d55f1..0943e7065e0e 100644 --- a/drivers/base/firmware_loader/main.c +++ b/drivers/base/firmware_loader/main.c @@ -631,6 +631,33 @@ request_firmware(const struct firmware **firmware_p, const char *name, } EXPORT_SYMBOL(request_firmware); +/** + * firmware_request_nowarn() - request for an optional fw module + * @firmware: pointer to firmware image + * @name: name of firmware file + * @device: device for which firmware is being loaded + * + * This function is similar in behaviour to request_firmware(), except + * it doesn't produce warning messages when the file is not found. + * The sysfs fallback mechanism is enabled if direct filesystem lookup fails, + * however, however failures to find the firmware file with it are still + * suppressed. It is therefore up to the driver to check for the return value + * of this call and to decide when to inform the users of errors. + **/ +int firmware_request_nowarn(const struct firmware **firmware, const char *name, + struct device *device) +{ + int ret; + + /* Need to pin this module until return */ + __module_get(THIS_MODULE); + ret = _request_firmware(firmware, name, device, NULL, 0, + FW_OPT_UEVENT | FW_OPT_NO_WARN); + module_put(THIS_MODULE); + return ret; +} +EXPORT_SYMBOL_GPL(firmware_request_nowarn); + /** * request_firmware_direct() - load firmware directly without usermode helper * @firmware_p: pointer to firmware image diff --git a/include/linux/firmware.h b/include/linux/firmware.h index 41050417cafb..2dd566c91d44 100644 --- a/include/linux/firmware.h +++ b/include/linux/firmware.h @@ -42,6 +42,8 @@ struct builtin_fw { #if defined(CONFIG_FW_LOADER) || (defined(CONFIG_FW_LOADER_MODULE) && defined(MODULE)) int request_firmware(const struct firmware **fw, const char *name, struct device *device); +int firmware_request_nowarn(const struct firmware **fw, const char *name, + struct device *device); int request_firmware_nowait( struct module *module, bool uevent, const char *name, struct device *device, gfp_t gfp, void *context, @@ -59,6 +61,14 @@ static inline int request_firmware(const struct firmware **fw, { return -EINVAL; } + +static inline int firmware_request_nowarn(const struct firmware **fw, + const char *name, + struct device *device) +{ + return -EINVAL; +} + static inline int request_firmware_nowait( struct module *module, bool uevent, const char *name, struct device *device, gfp_t gfp, void *context, From c8e028026387ea2d520a502971cfa21f8cc8212d Mon Sep 17 00:00:00 2001 From: Andres Rodriguez Date: Thu, 10 May 2018 13:08:46 -0700 Subject: [PATCH 12/23] ath10k: use firmware_request_nowarn() to load firmware This reduces the unnecessary spew when trying to load optional firmware: "Direct firmware load for ... failed with error -2" Signed-off-by: Andres Rodriguez Reviewed-by: Kees Cook Acked-by: Kalle Valo Signed-off-by: Luis R. Rodriguez Signed-off-by: Greg Kroah-Hartman --- drivers/net/wireless/ath/ath10k/core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/wireless/ath/ath10k/core.c b/drivers/net/wireless/ath/ath10k/core.c index 8a3020dbd4cf..936907227b9e 100644 --- a/drivers/net/wireless/ath/ath10k/core.c +++ b/drivers/net/wireless/ath/ath10k/core.c @@ -653,7 +653,7 @@ static const struct firmware *ath10k_fetch_fw_file(struct ath10k *ar, dir = "."; snprintf(filename, sizeof(filename), "%s/%s", dir, file); - ret = request_firmware(&fw, filename, ar->dev); + ret = firmware_request_nowarn(&fw, filename, ar->dev); ath10k_dbg(ar, ATH10K_DBG_BOOT, "boot fw request '%s': %d\n", filename, ret); From 1bc4d68b06ab913d392c8ad6481b9729bd58b8d5 Mon Sep 17 00:00:00 2001 From: Andres Rodriguez Date: Thu, 10 May 2018 13:08:47 -0700 Subject: [PATCH 13/23] ath10k: re-enable the firmware fallback mechanism for testmode The ath10k testmode uses request_firmware_direct() in order to avoid producing firmware load warnings. Disabling the fallback mechanism was a side effect of disabling warnings. We now have a new API that allows us to avoid warnings while keeping the fallback mechanism enabled. So use that instead. Signed-off-by: Andres Rodriguez Reviewed-by: Kees Cook Acked-by: Kalle Valo Signed-off-by: Luis R. Rodriguez Signed-off-by: Greg Kroah-Hartman --- drivers/net/wireless/ath/ath10k/testmode.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/net/wireless/ath/ath10k/testmode.c b/drivers/net/wireless/ath/ath10k/testmode.c index 568810b41657..c24ee616833c 100644 --- a/drivers/net/wireless/ath/ath10k/testmode.c +++ b/drivers/net/wireless/ath/ath10k/testmode.c @@ -157,7 +157,7 @@ static int ath10k_tm_fetch_utf_firmware_api_1(struct ath10k *ar, ar->hw_params.fw.dir, ATH10K_FW_UTF_FILE); /* load utf firmware image */ - ret = request_firmware_direct(&fw_file->firmware, filename, ar->dev); + ret = firmware_request_nowarn(&fw_file->firmware, filename, ar->dev); ath10k_dbg(ar, ATH10K_DBG_TESTMODE, "testmode fw request '%s': %d\n", filename, ret); From 4fde24d4161358e0909684facd2226627d381fce Mon Sep 17 00:00:00 2001 From: "Luis R. Rodriguez" Date: Thu, 10 May 2018 13:08:48 -0700 Subject: [PATCH 14/23] Documentation: fix few typos and clarifications for the firmware loader Fix a few typos, and clarify a few sentences. Reviewed-by: Kees Cook Signed-off-by: Luis R. Rodriguez Signed-off-by: Greg Kroah-Hartman --- Documentation/driver-api/firmware/fallback-mechanisms.rst | 5 +++-- Documentation/driver-api/firmware/firmware_cache.rst | 4 ++-- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/Documentation/driver-api/firmware/fallback-mechanisms.rst b/Documentation/driver-api/firmware/fallback-mechanisms.rst index f353783ae0be..a39323ef7d29 100644 --- a/Documentation/driver-api/firmware/fallback-mechanisms.rst +++ b/Documentation/driver-api/firmware/fallback-mechanisms.rst @@ -83,7 +83,7 @@ and a file upload firmware into: * /sys/$DEVPATH/data To upload firmware you will echo 1 onto the loading file to indicate -you are loading firmware. You then cat the firmware into the data file, +you are loading firmware. You then write the firmware into the data file, and you notify the kernel the firmware is ready by echo'ing 0 onto the loading file. @@ -136,7 +136,8 @@ by kobject uevents. This is specially exacerbated due to the fact that most distributions today disable CONFIG_FW_LOADER_USER_HELPER_FALLBACK. Refer to do_firmware_uevent() for details of the kobject event variables -setup. Variables passwdd with a kobject add event: +setup. The variables currently passed to userspace with a "kobject add" +event are: * FIRMWARE=firmware name * TIMEOUT=timeout value diff --git a/Documentation/driver-api/firmware/firmware_cache.rst b/Documentation/driver-api/firmware/firmware_cache.rst index 2210e5bfb332..c2e69d9c6bf1 100644 --- a/Documentation/driver-api/firmware/firmware_cache.rst +++ b/Documentation/driver-api/firmware/firmware_cache.rst @@ -29,8 +29,8 @@ Some implementation details about the firmware cache setup: * If an asynchronous call is used the firmware cache is only set up for a device if if the second argument (uevent) to request_firmware_nowait() is true. When uevent is true it requests that a kobject uevent be sent to - userspace for the firmware request. For details refer to the Fackback - mechanism documented below. + userspace for the firmware request through the sysfs fallback mechanism + if the firmware file is not found. * If the firmware cache is determined to be needed as per the above two criteria the firmware cache is setup by adding a devres entry for the From 0b92d3f6802609e5c5ec30571296c68cfb2cd4aa Mon Sep 17 00:00:00 2001 From: "Luis R. Rodriguez" Date: Thu, 10 May 2018 13:08:49 -0700 Subject: [PATCH 15/23] Documentation: remove stale firmware API reference It refers to a pending patch, but this was merged eons ago. Reviewed-by: Mauro Carvalho Chehab Reviewed-by: Kees Cook Signed-off-by: Luis R. Rodriguez Signed-off-by: Greg Kroah-Hartman --- Documentation/dell_rbu.txt | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/Documentation/dell_rbu.txt b/Documentation/dell_rbu.txt index 0fdb6aa2704c..5d1ce7bcd04d 100644 --- a/Documentation/dell_rbu.txt +++ b/Documentation/dell_rbu.txt @@ -121,10 +121,7 @@ read back the image downloaded. .. note:: - This driver requires a patch for firmware_class.c which has the modified - request_firmware_nowait function. - - Also after updating the BIOS image a user mode application needs to execute + After updating the BIOS image a user mode application needs to execute code which sends the BIOS update request to the BIOS. So on the next reboot the BIOS knows about the new image downloaded and it updates itself. Also don't unload the rbu driver if the image has to be updated. From f0a462970ee19930518b03798a21cfdb3fd35877 Mon Sep 17 00:00:00 2001 From: "Luis R. Rodriguez" Date: Thu, 10 May 2018 13:08:50 -0700 Subject: [PATCH 16/23] Documentation: clarify firmware_class provenance and why we can't rename the module Clarify the provenance of the firmware loader firmware_class module name and why we cannot rename the module in the future. Reviewed-by: Mauro Carvalho Chehab Reviewed-by: Kees Cook Signed-off-by: Luis R. Rodriguez Signed-off-by: Greg Kroah-Hartman --- .../driver-api/firmware/fallback-mechanisms.rst | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/Documentation/driver-api/firmware/fallback-mechanisms.rst b/Documentation/driver-api/firmware/fallback-mechanisms.rst index a39323ef7d29..d35fed65eae9 100644 --- a/Documentation/driver-api/firmware/fallback-mechanisms.rst +++ b/Documentation/driver-api/firmware/fallback-mechanisms.rst @@ -72,9 +72,12 @@ the firmware requested, and establishes it in the device hierarchy by associating the device used to make the request as the device's parent. The sysfs directory's file attributes are defined and controlled through the new device's class (firmware_class) and group (fw_dev_attr_groups). -This is actually where the original firmware_class.c file name comes from, -as originally the only firmware loading mechanism available was the -mechanism we now use as a fallback mechanism. +This is actually where the original firmware_class module name came from, +given that originally the only firmware loading mechanism available was the +mechanism we now use as a fallback mechanism, which registers a struct class +firmware_class. Because the attributes exposed are part of the module name, the +module name firmware_class cannot be renamed in the future, to ensure backward +compatibility with old userspace. To load firmware using the sysfs interface we expose a loading indicator, and a file upload firmware into: From 964f8363a1aba6cb4198bfaaac538b08f1c538f1 Mon Sep 17 00:00:00 2001 From: Andy Shevchenko Date: Thu, 3 May 2018 19:17:52 +0300 Subject: [PATCH 17/23] debugfs: Re-use kstrtobool_from_user() Re-use kstrtobool_from_user() instead of open coded variant. Signed-off-by: Andy Shevchenko Signed-off-by: Greg Kroah-Hartman --- fs/debugfs/file.c | 10 ++-------- 1 file changed, 2 insertions(+), 8 deletions(-) diff --git a/fs/debugfs/file.c b/fs/debugfs/file.c index 1f99678ff5d3..4fce1da7db23 100644 --- a/fs/debugfs/file.c +++ b/fs/debugfs/file.c @@ -796,19 +796,13 @@ EXPORT_SYMBOL_GPL(debugfs_read_file_bool); ssize_t debugfs_write_file_bool(struct file *file, const char __user *user_buf, size_t count, loff_t *ppos) { - char buf[32]; - size_t buf_size; bool bv; int r; bool *val = file->private_data; struct dentry *dentry = F_DENTRY(file); - buf_size = min(count, (sizeof(buf)-1)); - if (copy_from_user(buf, user_buf, buf_size)) - return -EFAULT; - - buf[buf_size] = '\0'; - if (strtobool(buf, &bv) == 0) { + r = kstrtobool_from_user(user_buf, count, &bv); + if (!r) { r = debugfs_file_get(dentry); if (unlikely(r)) return r; From 95cde3c59966f6371b6bcd9e4e2da2ba64ee9775 Mon Sep 17 00:00:00 2001 From: Thomas Richter Date: Fri, 27 Apr 2018 14:35:47 +0200 Subject: [PATCH 18/23] debugfs: inode: debugfs_create_dir uses mode permission from parent MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Currently function debugfs_create_dir() creates a new directory in the debugfs (usually mounted /sys/kernel/debug) with permission rwxr-xr-x. This is hard coded. Change this to use the parent directory permission. Output before the patch: root@s8360047 ~]# tree -dp -L 1 /sys/kernel/debug/ /sys/kernel/debug/ ├── [drwxr-xr-x] bdi ├── [drwxr-xr-x] block ├── [drwxr-xr-x] dasd ├── [drwxr-xr-x] device_component ├── [drwxr-xr-x] extfrag ├── [drwxr-xr-x] hid ├── [drwxr-xr-x] kprobes ├── [drwxr-xr-x] kvm ├── [drwxr-xr-x] memblock ├── [drwxr-xr-x] pm_qos ├── [drwxr-xr-x] qdio ├── [drwxr-xr-x] s390 ├── [drwxr-xr-x] s390dbf └── [drwx------] tracing 14 directories [root@s8360047 linux]# Output after the patch: [root@s8360047 ~]# tree -dp -L 1 /sys/kernel/debug/ sys/kernel/debug/ ├── [drwx------] bdi ├── [drwx------] block ├── [drwx------] dasd ├── [drwx------] device_component ├── [drwx------] extfrag ├── [drwx------] hid ├── [drwx------] kprobes ├── [drwx------] kvm ├── [drwx------] memblock ├── [drwx------] pm_qos ├── [drwx------] qdio ├── [drwx------] s390 ├── [drwx------] s390dbf └── [drwx------] tracing 14 directories [root@s8360047 linux]# Here is the full diff output done with: [root@s8360047 ~]# diff -u treefull.before treefull.after | sed 's-^- # -' > treefull.diff # --- treefull.before 2018-04-27 13:22:04.532824564 +0200 # +++ treefull.after 2018-04-27 13:24:12.106182062 +0200 # @@ -1,55 +1,55 @@ # /sys/kernel/debug/ # -├── [drwxr-xr-x] bdi # -│   ├── [drwxr-xr-x] 1:0 # -│   ├── [drwxr-xr-x] 1:1 # -│   ├── [drwxr-xr-x] 1:10 # -│   ├── [drwxr-xr-x] 1:11 # -│   ├── [drwxr-xr-x] 1:12 # -│   ├── [drwxr-xr-x] 1:13 # -│   ├── [drwxr-xr-x] 1:14 # -│   ├── [drwxr-xr-x] 1:15 # -│   ├── [drwxr-xr-x] 1:2 # -│   ├── [drwxr-xr-x] 1:3 # -│   ├── [drwxr-xr-x] 1:4 # -│   ├── [drwxr-xr-x] 1:5 # -│   ├── [drwxr-xr-x] 1:6 # -│   ├── [drwxr-xr-x] 1:7 # -│   ├── [drwxr-xr-x] 1:8 # -│   ├── [drwxr-xr-x] 1:9 # -│   └── [drwxr-xr-x] 94:0 # -├── [drwxr-xr-x] block # -├── [drwxr-xr-x] dasd # -│   ├── [drwxr-xr-x] 0.0.e18a # -│   ├── [drwxr-xr-x] dasda # -│   └── [drwxr-xr-x] global # -├── [drwxr-xr-x] device_component # -├── [drwxr-xr-x] extfrag # -├── [drwxr-xr-x] hid # -├── [drwxr-xr-x] kprobes # -├── [drwxr-xr-x] kvm # -├── [drwxr-xr-x] memblock # -├── [drwxr-xr-x] pm_qos # -├── [drwxr-xr-x] qdio # -│   └── [drwxr-xr-x] 0.0.f5f2 # -├── [drwxr-xr-x] s390 # -│   └── [drwxr-xr-x] stsi # -├── [drwxr-xr-x] s390dbf # -│   ├── [drwxr-xr-x] 0.0.e18a # -│   ├── [drwxr-xr-x] cio_crw # -│   ├── [drwxr-xr-x] cio_msg # -│   ├── [drwxr-xr-x] cio_trace # -│   ├── [drwxr-xr-x] dasd # -│   ├── [drwxr-xr-x] kvm-trace # -│   ├── [drwxr-xr-x] lgr # -│   ├── [drwxr-xr-x] qdio_0.0.f5f2 # -│   ├── [drwxr-xr-x] qdio_error # -│   ├── [drwxr-xr-x] qdio_setup # -│   ├── [drwxr-xr-x] qeth_card_0.0.f5f0 # -│   ├── [drwxr-xr-x] qeth_control # -│   ├── [drwxr-xr-x] qeth_msg # -│   ├── [drwxr-xr-x] qeth_setup # -│   ├── [drwxr-xr-x] vmcp # -│   └── [drwxr-xr-x] vmur # +├── [drwx------] bdi # +│   ├── [drwx------] 1:0 # +│   ├── [drwx------] 1:1 # +│   ├── [drwx------] 1:10 # +│   ├── [drwx------] 1:11 # +│   ├── [drwx------] 1:12 # +│   ├── [drwx------] 1:13 # +│   ├── [drwx------] 1:14 # +│   ├── [drwx------] 1:15 # +│   ├── [drwx------] 1:2 # +│   ├── [drwx------] 1:3 # +│   ├── [drwx------] 1:4 # +│   ├── [drwx------] 1:5 # +│   ├── [drwx------] 1:6 # +│   ├── [drwx------] 1:7 # +│   ├── [drwx------] 1:8 # +│   ├── [drwx------] 1:9 # +│   └── [drwx------] 94:0 # +├── [drwx------] block # +├── [drwx------] dasd # +│   ├── [drwx------] 0.0.e18a # +│   ├── [drwx------] dasda # +│   └── [drwx------] global # +├── [drwx------] device_component # +├── [drwx------] extfrag # +├── [drwx------] hid # +├── [drwx------] kprobes # +├── [drwx------] kvm # +├── [drwx------] memblock # +├── [drwx------] pm_qos # +├── [drwx------] qdio # +│   └── [drwx------] 0.0.f5f2 # +├── [drwx------] s390 # +│   └── [drwx------] stsi # +├── [drwx------] s390dbf # +│   ├── [drwx------] 0.0.e18a # +│   ├── [drwx------] cio_crw # +│   ├── [drwx------] cio_msg # +│   ├── [drwx------] cio_trace # +│   ├── [drwx------] dasd # +│   ├── [drwx------] kvm-trace # +│   ├── [drwx------] lgr # +│   ├── [drwx------] qdio_0.0.f5f2 # +│   ├── [drwx------] qdio_error # +│   ├── [drwx------] qdio_setup # +│   ├── [drwx------] qeth_card_0.0.f5f0 # +│   ├── [drwx------] qeth_control # +│   ├── [drwx------] qeth_msg # +│   ├── [drwx------] qeth_setup # +│   ├── [drwx------] vmcp # +│   └── [drwx------] vmur # └── [drwx------] tracing # ├── [drwxr-xr-x] events # │   ├── [drwxr-xr-x] alarmtimer Fixes: edac65eaf8d5c ("debugfs: take mode-dependent parts of debugfs_get_inode() into callers") Signed-off-by: Thomas Richter Reviewed-by: Kees Cook Signed-off-by: Greg Kroah-Hartman --- fs/debugfs/inode.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/fs/debugfs/inode.c b/fs/debugfs/inode.c index 13b01351dd1c..a913b12fc7f8 100644 --- a/fs/debugfs/inode.c +++ b/fs/debugfs/inode.c @@ -512,7 +512,9 @@ struct dentry *debugfs_create_dir(const char *name, struct dentry *parent) if (unlikely(!inode)) return failed_creating(dentry); - inode->i_mode = S_IFDIR | S_IRWXU | S_IRUGO | S_IXUGO; + if (!parent) + parent = debugfs_mount->mnt_root; + inode->i_mode = S_IFDIR | ((d_inode(parent)->i_mode & 0770)); inode->i_op = &simple_dir_inode_operations; inode->i_fop = &simple_dir_operations; From 13509860efcada3e57c16d5f2e60dda8cef6054c Mon Sep 17 00:00:00 2001 From: Wolfram Sang Date: Sun, 6 May 2018 13:23:47 +0200 Subject: [PATCH 19/23] base: core: fix typo 'can by' to 'can be' Signed-off-by: Wolfram Sang Signed-off-by: Greg Kroah-Hartman --- drivers/base/core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/base/core.c b/drivers/base/core.c index d680fd030316..c4fc083870c2 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -2689,7 +2689,7 @@ static int device_move_class_links(struct device *dev, /** * device_move - moves a device to a new parent * @dev: the pointer to the struct device to be moved - * @new_parent: the new parent of the device (can by NULL) + * @new_parent: the new parent of the device (can be NULL) * @dpm_order: how to reorder the dpm_list */ int device_move(struct device *dev, struct device *new_parent, From 085aa2de568493d7cde52126512d37260077811a Mon Sep 17 00:00:00 2001 From: Arvind Yadav Date: Thu, 26 Apr 2018 21:12:09 +0530 Subject: [PATCH 20/23] mm: memory_hotplug: use put_device() if device_register fail if device_register() returned an error. Always use put_device() to give up the initialized reference and release allocated memory. Signed-off-by: Arvind Yadav Signed-off-by: Greg Kroah-Hartman --- drivers/base/memory.c | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/drivers/base/memory.c b/drivers/base/memory.c index bffe8616bd55..f5e560188a18 100644 --- a/drivers/base/memory.c +++ b/drivers/base/memory.c @@ -649,13 +649,19 @@ static const struct attribute_group *memory_memblk_attr_groups[] = { static int register_memory(struct memory_block *memory) { + int ret; + memory->dev.bus = &memory_subsys; memory->dev.id = memory->start_section_nr / sections_per_block; memory->dev.release = memory_block_release; memory->dev.groups = memory_memblk_attr_groups; memory->dev.offline = memory->state == MEM_OFFLINE; - return device_register(&memory->dev); + ret = device_register(&memory->dev); + if (ret) + put_device(&memory->dev); + + return ret; } static int init_memory_block(struct memory_block **memory, From 6a8b55d7f2265efdabf257211b662400615cf580 Mon Sep 17 00:00:00 2001 From: Mathieu Malaterre Date: Sat, 5 May 2018 21:57:41 +0200 Subject: [PATCH 21/23] driver core: add __printf verification to device_create_groups_vargs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit __printf is useful to verify format and arguments. Remove the following warning (with W=1): drivers/base/core.c:2435:2: warning: function might be possible candidate for ‘gnu_printf’ format attribute [-Wsuggest-attribute=format] Signed-off-by: Mathieu Malaterre Signed-off-by: Greg Kroah-Hartman --- drivers/base/core.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/drivers/base/core.c b/drivers/base/core.c index c4fc083870c2..35221144e0e6 100644 --- a/drivers/base/core.c +++ b/drivers/base/core.c @@ -2411,7 +2411,7 @@ static void device_create_release(struct device *dev) kfree(dev); } -static struct device * +static __printf(6, 0) struct device * device_create_groups_vargs(struct class *class, struct device *parent, dev_t devt, void *drvdata, const struct attribute_group **groups, From 0dda2bb6242361afd68332bf19bd67cd5981eb26 Mon Sep 17 00:00:00 2001 From: Florian Schmaus Date: Wed, 23 May 2018 17:59:11 +0200 Subject: [PATCH 22/23] driver-core: return EINVAL error instead of BUG_ON() I triggerd the BUG_ON() in driver_register() when booting a domU Xen domain. Since there was no contextual information logged, I needed to attach kgdb to determine the culprit (the wmi-bmof driver in my case). The BUG_ON() was added in commit f48f3febb2cb ("driver-core: do not register a driver with bus_type not registered"). Instead of running into a BUG_ON() we print an error message identifying the, likely faulty, driver but continue booting. Signed-off-by: Florian Schmaus Signed-off-by: Greg Kroah-Hartman --- drivers/base/driver.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/drivers/base/driver.c b/drivers/base/driver.c index ba912558a510..857c8f1b876e 100644 --- a/drivers/base/driver.c +++ b/drivers/base/driver.c @@ -148,7 +148,11 @@ int driver_register(struct device_driver *drv) int ret; struct device_driver *other; - BUG_ON(!drv->bus->p); + if (!drv->bus->p) { + pr_err("Driver '%s' was unable to register with bus_type '%s' because the bus was not initialized.\n", + drv->name, drv->bus->name); + return -EINVAL; + } if ((drv->bus->probe && drv->probe) || (drv->bus->remove && drv->remove) || From 8c97a46af04b4f7c0a0dded031fef1806872e648 Mon Sep 17 00:00:00 2001 From: Martin Liu Date: Thu, 31 May 2018 00:31:36 +0800 Subject: [PATCH 23/23] driver core: hold dev's parent lock when needed SoC have internal I/O buses that can't be proved for devices. The devices on the buses can be accessed directly without additinal configuration required. This type of bus is represented as "simple-bus". In some platforms, we name "soc" with "simple-bus" attribute and many devices are hooked under it described in DT (device tree). In commit bf74ad5bc417 ("Hold the device's parent's lock during probe and remove") to solve USB subsystem lock sequence since USB device's characteristic. Thus "soc" needs to be locked whenever a device and driver's probing happen under "soc" bus. During this period, an async driver tries to probe a device which is under the "soc" bus would be blocked until previous driver finish the probing and release "soc" lock. And the next probing under the "soc" bus need to wait for async finish. Because of that, driver's async probe for init time improvement will be shadowed. Since many devices don't have USB devices' characteristic, they actually don't need parent's lock. Thus, we introduce a lock flag in bus_type struct and driver core would lock the parent lock base on the flag. For USB, we set this flag in usb_bus_type to keep original lock behavior in driver core. Async probe could have more benefit after this patch. Signed-off-by: Martin Liu Acked-by: Alan Stern Signed-off-by: Greg Kroah-Hartman --- drivers/base/bus.c | 16 ++++++++-------- drivers/base/dd.c | 8 ++++---- drivers/usb/core/driver.c | 1 + include/linux/device.h | 3 +++ 4 files changed, 16 insertions(+), 12 deletions(-) diff --git a/drivers/base/bus.c b/drivers/base/bus.c index ef6183306b40..8bfd27ec73d6 100644 --- a/drivers/base/bus.c +++ b/drivers/base/bus.c @@ -184,10 +184,10 @@ static ssize_t unbind_store(struct device_driver *drv, const char *buf, dev = bus_find_device_by_name(bus, NULL, buf); if (dev && dev->driver == drv) { - if (dev->parent) /* Needed for USB */ + if (dev->parent && dev->bus->need_parent_lock) device_lock(dev->parent); device_release_driver(dev); - if (dev->parent) + if (dev->parent && dev->bus->need_parent_lock) device_unlock(dev->parent); err = count; } @@ -211,12 +211,12 @@ static ssize_t bind_store(struct device_driver *drv, const char *buf, dev = bus_find_device_by_name(bus, NULL, buf); if (dev && dev->driver == NULL && driver_match_device(drv, dev)) { - if (dev->parent) /* Needed for USB */ + if (dev->parent && bus->need_parent_lock) device_lock(dev->parent); device_lock(dev); err = driver_probe_device(drv, dev); device_unlock(dev); - if (dev->parent) + if (dev->parent && bus->need_parent_lock) device_unlock(dev->parent); if (err > 0) { @@ -735,10 +735,10 @@ static int __must_check bus_rescan_devices_helper(struct device *dev, int ret = 0; if (!dev->driver) { - if (dev->parent) /* Needed for USB */ + if (dev->parent && dev->bus->need_parent_lock) device_lock(dev->parent); ret = device_attach(dev); - if (dev->parent) + if (dev->parent && dev->bus->need_parent_lock) device_unlock(dev->parent); } return ret < 0 ? ret : 0; @@ -770,10 +770,10 @@ EXPORT_SYMBOL_GPL(bus_rescan_devices); int device_reprobe(struct device *dev) { if (dev->driver) { - if (dev->parent) /* Needed for USB */ + if (dev->parent && dev->bus->need_parent_lock) device_lock(dev->parent); device_release_driver(dev); - if (dev->parent) + if (dev->parent && dev->bus->need_parent_lock) device_unlock(dev->parent); } return bus_rescan_devices_helper(dev, NULL); diff --git a/drivers/base/dd.c b/drivers/base/dd.c index c9f54089429b..7c09f73b96f3 100644 --- a/drivers/base/dd.c +++ b/drivers/base/dd.c @@ -817,13 +817,13 @@ static int __driver_attach(struct device *dev, void *data) return ret; } /* ret > 0 means positive match */ - if (dev->parent) /* Needed for USB */ + if (dev->parent && dev->bus->need_parent_lock) device_lock(dev->parent); device_lock(dev); if (!dev->driver) driver_probe_device(drv, dev); device_unlock(dev); - if (dev->parent) + if (dev->parent && dev->bus->need_parent_lock) device_unlock(dev->parent); return 0; @@ -919,7 +919,7 @@ void device_release_driver_internal(struct device *dev, struct device_driver *drv, struct device *parent) { - if (parent) + if (parent && dev->bus->need_parent_lock) device_lock(parent); device_lock(dev); @@ -927,7 +927,7 @@ void device_release_driver_internal(struct device *dev, __device_release_driver(dev, parent); device_unlock(dev); - if (parent) + if (parent && dev->bus->need_parent_lock) device_unlock(parent); } diff --git a/drivers/usb/core/driver.c b/drivers/usb/core/driver.c index 9792cedfc351..e76e95f62f76 100644 --- a/drivers/usb/core/driver.c +++ b/drivers/usb/core/driver.c @@ -1922,4 +1922,5 @@ struct bus_type usb_bus_type = { .name = "usb", .match = usb_device_match, .uevent = usb_uevent, + .need_parent_lock = true, }; diff --git a/include/linux/device.h b/include/linux/device.h index 477956990f5e..beca424395dd 100644 --- a/include/linux/device.h +++ b/include/linux/device.h @@ -98,6 +98,8 @@ extern void bus_remove_file(struct bus_type *, struct bus_attribute *); * @lock_key: Lock class key for use by the lock validator * @force_dma: Assume devices on this bus should be set up by dma_configure() * even if DMA capability is not explicitly described by firmware. + * @need_parent_lock: When probing or removing a device on this bus, the + * device core should lock the device's parent. * * A bus is a channel between the processor and one or more devices. For the * purposes of the device model, all devices are connected via a bus, even if @@ -138,6 +140,7 @@ struct bus_type { struct lock_class_key lock_key; bool force_dma; + bool need_parent_lock; }; extern int __must_check bus_register(struct bus_type *bus);