Bluetooth: qca: fix firmware check error path

Commit Message

Johan Hovold May 1, 2024, 6:37 a.m. UTC

A recent commit fixed the code that parses the firmware files before
downloading them to the controller but introduced a memory leak in case
the sanity checks ever fail.

Make sure to free the firmware buffer before returning on errors.

Fixes: 6fb81c405bfa ("Bluetooth: qca: add missing firmware sanity checks")
Cc: [e-mail]      # 4.19
Signed-off-by: Johan Hovold <[e-mail]>
---
 drivers/bluetooth/btqca.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Luiz,

Sorry for not catching this before posting v2.

Feel to free to squash this fix into commit 6fb81c405bfa ("Bluetooth:
qca: add missing firmware sanity checks") in the bluetooth-next branch
if you prefer.

Note that the commit id in the Fixes tag above won't match if you cherry
pick both commits to the fixes branch.

Johan

Comments

[e-mail] May 1, 2024, 7:33 a.m. UTC | #1

This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=849572

---Test result---

Test Summary:
CheckPatch                    PASS      0.68 seconds
GitLint                       PASS      0.34 seconds
SubjectPrefix                 PASS      0.13 seconds
BuildKernel                   PASS      30.20 seconds
CheckAllWarning               PASS      32.88 seconds
CheckSparse                   PASS      38.56 seconds
CheckSmatch                   FAIL      36.25 seconds
BuildKernel32                 PASS      29.14 seconds
TestRunnerSetup               PASS      521.38 seconds
TestRunner_l2cap-tester       PASS      20.56 seconds
TestRunner_iso-tester         PASS      31.65 seconds
TestRunner_bnep-tester        PASS      4.74 seconds
TestRunner_mgmt-tester        FAIL      109.86 seconds
TestRunner_rfcomm-tester      PASS      7.35 seconds
TestRunner_sco-tester         PASS      15.00 seconds
TestRunner_ioctl-tester       PASS      7.72 seconds
TestRunner_mesh-tester        PASS      5.85 seconds
TestRunner_smp-tester         PASS      6.74 seconds
TestRunner_userchan-tester    PASS      4.98 seconds
IncrementalBuild              PASS      28.64 seconds

Details
##############################
Test: CheckSmatch - FAIL
Desc: Run smatch tool with source
Output:

Segmentation fault (core dumped)
make[4]: *** [scripts/Makefile.build:244: net/bluetooth/hci_core.o] Error 139
make[4]: *** Deleting file 'net/bluetooth/hci_core.o'
make[3]: *** [scripts/Makefile.build:485: net/bluetooth] Error 2
make[2]: *** [scripts/Makefile.build:485: net] Error 2
make[2]: *** Waiting for unfinished jobs....
Segmentation fault (core dumped)
make[4]: *** [scripts/Makefile.build:244: drivers/bluetooth/bcm203x.o] Error 139
make[4]: *** Deleting file 'drivers/bluetooth/bcm203x.o'
make[4]: *** Waiting for unfinished jobs....
Segmentation fault (core dumped)
make[4]: *** [scripts/Makefile.build:244: drivers/bluetooth/bpa10x.o] Error 139
make[4]: *** Deleting file 'drivers/bluetooth/bpa10x.o'
make[3]: *** [scripts/Makefile.build:485: drivers/bluetooth] Error 2
make[2]: *** [scripts/Makefile.build:485: drivers] Error 2
make[1]: *** [/github/workspace/src/src/Makefile:1919: .] Error 2
make: *** [Makefile:240: __sub-make] Error 2
##############################
Test: TestRunner_mgmt-tester - FAIL
Desc: Run mgmt-tester with test-runner
Output:
Total: 492, Passed: 487 (99.0%), Failed: 3, Not Run: 2

Failed Test Cases
LL Privacy - Add Device 5 (2 Devices to RL)          Failed       0.155 seconds
LL Privacy - Add Device 7 (AL is full)               Failed       0.196 seconds
LL Privacy - Start Discovery 2 (Disable RL)          Failed       0.175 seconds


---
Regards,
Linux Bluetooth

[e-mail] May 3, 2024, 5 p.m. UTC | #2

Hello:

This patch was applied to bluetooth/bluetooth-next.git (master)
by Luiz Augusto von Dentz <[e-mail]>:

On Wed,  1 May 2024 08:37:40 +0200 you wrote:
> A recent commit fixed the code that parses the firmware files before
> downloading them to the controller but introduced a memory leak in case
> the sanity checks ever fail.
> 
> Make sure to free the firmware buffer before returning on errors.
> 
> Fixes: 6fb81c405bfa ("Bluetooth: qca: add missing firmware sanity checks")
> Cc: [e-mail]      # 4.19
> Signed-off-by: Johan Hovold <[e-mail]>
> 
> [...]

Here is the summary with links:
  - Bluetooth: qca: fix firmware check error path
    https://git.kernel.org/bluetooth/bluetooth-next/c/8da121d4396b

You are awesome, thank you!

Patch

diff --git a/drivers/bluetooth/btqca.c b/drivers/bluetooth/btqca.c
index 125f140e012a..59e384fa1a0c 100644
--- a/drivers/bluetooth/btqca.c
+++ b/drivers/bluetooth/btqca.c
@@ -676,7 +676,7 @@  static int qca_download_firmware(struct hci_dev *hdev,
 
 	ret = qca_tlv_check_data(hdev, config, data, size, soc_type);
 	if (ret)
-		return ret;
+		goto out;
 
 	segment = data;
 	remain = size;