[PATCH] Bluetooth: hci_ll: Fix firmware leak on error path

Anas Iqbal posted 1 patch 3 weeks, 2 days ago
There is a newer version of this series
drivers/bluetooth/hci_ll.c | 2 ++
1 file changed, 2 insertions(+)
[PATCH] Bluetooth: hci_ll: Fix firmware leak on error path
Posted by Anas Iqbal 3 weeks, 2 days ago
Smatch reports:
drivers/bluetooth/hci_ll.c:587 download_firmware() warn:
'fw' from request_firmware() not released on lines: 544.

In download_firmware(), if request_firmware() succeeds but the returned
firmware has no data or size, the function returns immediately without
releasing the firmware, resulting in a resource leak.

Add a release_firmware() call before returning when request_firmware()
succeeds but the firmware contents are invalid.

Signed-off-by: Anas Iqbal <mohd.abd.6602@gmail.com>
---
 drivers/bluetooth/hci_ll.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/bluetooth/hci_ll.c b/drivers/bluetooth/hci_ll.c
index 91acf24f1ef5..91c96ad12342 100644
--- a/drivers/bluetooth/hci_ll.c
+++ b/drivers/bluetooth/hci_ll.c
@@ -541,6 +541,8 @@ static int download_firmware(struct ll_device *lldev)
 	if (err || !fw->data || !fw->size) {
 		bt_dev_err(lldev->hu.hdev, "request_firmware failed(errno %d) for %s",
 			   err, bts_scr_name);
+		if (!err)
+			release_firmware(fw);
 		return -EINVAL;
 	}
 	ptr = (void *)fw->data;
-- 
2.43.0
Re: [PATCH] Bluetooth: hci_ll: Fix firmware leak on error path
Posted by Paul Menzel 3 weeks, 1 day ago
Dear Anas,


Thank you for your patch.

Am 14.03.26 um 17:56 schrieb Anas Iqbal:
> Smatch reports:
> drivers/bluetooth/hci_ll.c:587 download_firmware() warn:
> 'fw' from request_firmware() not released on lines: 544.
> 
> In download_firmware(), if request_firmware() succeeds but the returned
> firmware has no data or size, the function returns immediately without
> releasing the firmware, resulting in a resource leak.
> 
> Add a release_firmware() call before returning when request_firmware()
> succeeds but the firmware contents are invalid.

Change to *content is*.

> Signed-off-by: Anas Iqbal <mohd.abd.6602@gmail.com>

Please also add a Fixes: tag.

> ---
>   drivers/bluetooth/hci_ll.c | 2 ++
>   1 file changed, 2 insertions(+)
> 
> diff --git a/drivers/bluetooth/hci_ll.c b/drivers/bluetooth/hci_ll.c
> index 91acf24f1ef5..91c96ad12342 100644
> --- a/drivers/bluetooth/hci_ll.c
> +++ b/drivers/bluetooth/hci_ll.c
> @@ -541,6 +541,8 @@ static int download_firmware(struct ll_device *lldev)
>   	if (err || !fw->data || !fw->size) {
>   		bt_dev_err(lldev->hu.hdev, "request_firmware failed(errno %d) for %s",
>   			   err, bts_scr_name);
> +		if (!err)
> +			release_firmware(fw);
>   		return -EINVAL;
>   	}
>   	ptr = (void *)fw->data;

With the improved commit message, feel free to add:

Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de>


Kind regards,

Paul
[PATCH v2] Bluetooth: hci_ll: Fix firmware leak on error path
Posted by Anas Iqbal 3 weeks, 1 day ago
Smatch reports:

drivers/bluetooth/hci_ll.c:587 download_firmware() warn:
'fw' from request_firmware() not released on lines: 544.

In download_firmware(), if request_firmware() succeeds but the returned
firmware content is invalid (no data or zero size), the function returns
without releasing the firmware, resulting in a resource leak.

Fix this by calling release_firmware() before returning when
request_firmware() succeeded but the firmware content is invalid.

Fixes: 371805522f87 ("bluetooth: hci_uart: add LL protocol serdev driver support")
Reviewed-by: Paul Menzel <pmenzel@molgen.mpg.de>
Signed-off-by: Anas Iqbal <mohd.abd.6602@gmail.com>
---
v2:
 - Fix grammar ("content is")
 - Add Fixes tag
 - Add Reviewed-by tag from Paul Menzel
---
 drivers/bluetooth/hci_ll.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/drivers/bluetooth/hci_ll.c b/drivers/bluetooth/hci_ll.c
index 91acf24f1ef5..91c96ad12342 100644
--- a/drivers/bluetooth/hci_ll.c
+++ b/drivers/bluetooth/hci_ll.c
@@ -541,6 +541,8 @@ static int download_firmware(struct ll_device *lldev)
 	if (err || !fw->data || !fw->size) {
 		bt_dev_err(lldev->hu.hdev, "request_firmware failed(errno %d) for %s",
 			   err, bts_scr_name);
+		if (!err)
+			release_firmware(fw);
 		return -EINVAL;
 	}
 	ptr = (void *)fw->data;
-- 
2.43.0