drivers/base/firmware_loader/sysfs_upload.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-)
firmware_upload_register()
-> fw_create_instance()
-> device_initialize()
After fw_create_instance() succeeds, the lifetime of the embedded struct
device is expected to be managed through the device core reference
counting, since fw_create_instance() has already called
device_initialize().
In firmware_upload_register(), if alloc_lookup_fw_priv() fails after
fw_create_instance() succeeds, the code reaches free_fw_sysfs and frees
fw_sysfs directly instead of releasing the device reference with
put_device(). This may leave the reference count of the embedded struct
device unbalanced, resulting in a refcount leak.
The issue was identified by a static analysis tool I developed and
confirmed by manual review. Fix this by using put_device(fw_dev) in the
failure path and letting fw_dev_release() handle the final cleanup,
instead of freeing the instance directly from the error path.
Fixes: 97730bbb242c ("firmware_loader: Add firmware-upload support")
Cc: stable@vger.kernel.org
Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
---
drivers/base/firmware_loader/sysfs_upload.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/drivers/base/firmware_loader/sysfs_upload.c b/drivers/base/firmware_loader/sysfs_upload.c
index f59a7856934c..6b701185dcb6 100644
--- a/drivers/base/firmware_loader/sysfs_upload.c
+++ b/drivers/base/firmware_loader/sysfs_upload.c
@@ -366,7 +366,8 @@ firmware_upload_register(struct module *module, struct device *parent,
return fw_upload;
free_fw_sysfs:
- kfree(fw_sysfs);
+ put_device(fw_dev);
+ goto exit_module_put;
free_fw_upload_priv:
kfree(fw_upload_priv);
--
2.43.0
On Wed, Apr 15, 2026 at 04:51:09PM +0800, Guangshuo Li wrote:
> firmware_upload_register()
> -> fw_create_instance()
> -> device_initialize()
>
> After fw_create_instance() succeeds, the lifetime of the embedded struct
> device is expected to be managed through the device core reference
> counting, since fw_create_instance() has already called
> device_initialize().
>
> In firmware_upload_register(), if alloc_lookup_fw_priv() fails after
> fw_create_instance() succeeds, the code reaches free_fw_sysfs and frees
> fw_sysfs directly instead of releasing the device reference with
> put_device(). This may leave the reference count of the embedded struct
> device unbalanced, resulting in a refcount leak.
>
> The issue was identified by a static analysis tool I developed and
> confirmed by manual review. Fix this by using put_device(fw_dev) in the
> failure path and letting fw_dev_release() handle the final cleanup,
> instead of freeing the instance directly from the error path.
>
> Fixes: 97730bbb242c ("firmware_loader: Add firmware-upload support")
> Cc: stable@vger.kernel.org
> Signed-off-by: Guangshuo Li <lgs201920130244@gmail.com>
> ---
> drivers/base/firmware_loader/sysfs_upload.c | 3 ++-
> 1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/drivers/base/firmware_loader/sysfs_upload.c b/drivers/base/firmware_loader/sysfs_upload.c
> index f59a7856934c..6b701185dcb6 100644
> --- a/drivers/base/firmware_loader/sysfs_upload.c
> +++ b/drivers/base/firmware_loader/sysfs_upload.c
> @@ -366,7 +366,8 @@ firmware_upload_register(struct module *module, struct device *parent,
> return fw_upload;
>
> free_fw_sysfs:
> - kfree(fw_sysfs);
> + put_device(fw_dev);
> + goto exit_module_put;
Thanks for the patch!
Given that the free_fw_sysfs target is used only once and no longer
falls through, I suggest we remove the free_fw_sysfs target
altogether.
Instead of:
goto free_fw_sysfs;
Do:
put_device(fw_dev);
goto exit_module_put;
- Russ
>
> free_fw_upload_priv:
> kfree(fw_upload_priv);
> --
> 2.43.0
>
Hi Russ, Thanks for reviewing. On Sat, 18 Apr 2026 at 07:22, Russ Weight <russ.weight@linux.dev> wrote: > > Given that the free_fw_sysfs target is used only once and no longer > falls through, I suggest we remove the free_fw_sysfs target > altogether. > > Instead of: > goto free_fw_sysfs; > > Do: > put_device(fw_dev); > goto exit_module_put; > > - Russ > > > > > free_fw_upload_priv: > > kfree(fw_upload_priv); > > -- > > 2.43.0 > > Good point — I agree that since free_fw_sysfs is now only used once and no longer falls through, it makes sense to remove that label entirely and switch the error path to put_device(fw_dev); goto exit_module_put; directly. I'll send v2 shortly. Thanks, Guangshuo
© 2016 - 2026 Red Hat, Inc.