From 8b40c38e37492b5bdf8e95b46b5cca9517a9957a Mon Sep 17 00:00:00 2001 From: Russ Weight Date: Mon, 29 Aug 2022 10:45:57 -0700 Subject: firmware_loader: Fix use-after-free during unregister In the following code within firmware_upload_unregister(), the call to device_unregister() could result in the dev_release function freeing the fw_upload_priv structure before it is dereferenced for the call to module_put(). This bug was found by the kernel test robot using CONFIG_KASAN while running the firmware selftests. device_unregister(&fw_sysfs->dev); module_put(fw_upload_priv->module); The problem is fixed by copying fw_upload_priv->module to a local variable for use when calling device_unregister(). Fixes: 97730bbb242c ("firmware_loader: Add firmware-upload support") Cc: stable Reported-by: kernel test robot Reviewed-by: Matthew Gerlach Signed-off-by: Russ Weight Link: https://lore.kernel.org/r/20220829174557.437047-1-russell.h.weight@intel.com Signed-off-by: Greg Kroah-Hartman --- drivers/base/firmware_loader/sysfs_upload.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) (limited to 'drivers/base/firmware_loader/sysfs_upload.c') diff --git a/drivers/base/firmware_loader/sysfs_upload.c b/drivers/base/firmware_loader/sysfs_upload.c index 87044d52322a..63e15bddd80c 100644 --- a/drivers/base/firmware_loader/sysfs_upload.c +++ b/drivers/base/firmware_loader/sysfs_upload.c @@ -377,6 +377,7 @@ void firmware_upload_unregister(struct fw_upload *fw_upload) { struct fw_sysfs *fw_sysfs = fw_upload->priv; struct fw_upload_priv *fw_upload_priv = fw_sysfs->fw_upload_priv; + struct module *module = fw_upload_priv->module; mutex_lock(&fw_upload_priv->lock); if (fw_upload_priv->progress == FW_UPLOAD_PROG_IDLE) { @@ -392,6 +393,6 @@ void firmware_upload_unregister(struct fw_upload *fw_upload) unregister: device_unregister(&fw_sysfs->dev); - module_put(fw_upload_priv->module); + module_put(module); } EXPORT_SYMBOL_GPL(firmware_upload_unregister); -- cgit From 789bba82f63c3e81dce426ba457fc7905b30ac6e Mon Sep 17 00:00:00 2001 From: Russ Weight Date: Tue, 30 Aug 2022 17:25:18 -0700 Subject: firmware_loader: Fix memory leak in firmware upload In the case of firmware-upload, an instance of struct fw_upload is allocated in firmware_upload_register(). This data needs to be freed in fw_dev_release(). Create a new fw_upload_free() function in sysfs_upload.c to handle the firmware-upload specific memory frees and incorporate the missing kfree call for the fw_upload structure. Fixes: 97730bbb242c ("firmware_loader: Add firmware-upload support") Cc: stable Signed-off-by: Russ Weight Link: https://lore.kernel.org/r/20220831002518.465274-1-russell.h.weight@intel.com Signed-off-by: Greg Kroah-Hartman --- drivers/base/firmware_loader/sysfs.c | 7 +++---- drivers/base/firmware_loader/sysfs.h | 5 +++++ drivers/base/firmware_loader/sysfs_upload.c | 9 +++++++++ 3 files changed, 17 insertions(+), 4 deletions(-) (limited to 'drivers/base/firmware_loader/sysfs_upload.c') diff --git a/drivers/base/firmware_loader/sysfs.c b/drivers/base/firmware_loader/sysfs.c index 77bad32c481a..5b66b3d1fa16 100644 --- a/drivers/base/firmware_loader/sysfs.c +++ b/drivers/base/firmware_loader/sysfs.c @@ -93,10 +93,9 @@ static void fw_dev_release(struct device *dev) { struct fw_sysfs *fw_sysfs = to_fw_sysfs(dev); - if (fw_sysfs->fw_upload_priv) { - free_fw_priv(fw_sysfs->fw_priv); - kfree(fw_sysfs->fw_upload_priv); - } + if (fw_sysfs->fw_upload_priv) + fw_upload_free(fw_sysfs); + kfree(fw_sysfs); } diff --git a/drivers/base/firmware_loader/sysfs.h b/drivers/base/firmware_loader/sysfs.h index 5d8ff1675c79..df1d5add698f 100644 --- a/drivers/base/firmware_loader/sysfs.h +++ b/drivers/base/firmware_loader/sysfs.h @@ -106,12 +106,17 @@ extern struct device_attribute dev_attr_cancel; extern struct device_attribute dev_attr_remaining_size; int fw_upload_start(struct fw_sysfs *fw_sysfs); +void fw_upload_free(struct fw_sysfs *fw_sysfs); umode_t fw_upload_is_visible(struct kobject *kobj, struct attribute *attr, int n); #else static inline int fw_upload_start(struct fw_sysfs *fw_sysfs) { return 0; } + +static inline void fw_upload_free(struct fw_sysfs *fw_sysfs) +{ +} #endif #endif /* __FIRMWARE_SYSFS_H */ diff --git a/drivers/base/firmware_loader/sysfs_upload.c b/drivers/base/firmware_loader/sysfs_upload.c index 63e15bddd80c..a0af8f5f13d8 100644 --- a/drivers/base/firmware_loader/sysfs_upload.c +++ b/drivers/base/firmware_loader/sysfs_upload.c @@ -264,6 +264,15 @@ int fw_upload_start(struct fw_sysfs *fw_sysfs) return 0; } +void fw_upload_free(struct fw_sysfs *fw_sysfs) +{ + struct fw_upload_priv *fw_upload_priv = fw_sysfs->fw_upload_priv; + + free_fw_priv(fw_sysfs->fw_priv); + kfree(fw_upload_priv->fw_upload); + kfree(fw_upload_priv); +} + /** * firmware_upload_register() - register for the firmware upload sysfs API * @module: kernel module of this device -- cgit