[PATCH 0/2] Bluetooth: btrtl: Fix NULL return on errors in btrtl_initialize()

[PATCH 0/2] Bluetooth: btrtl: Fix NULL return on errors in btrtl_initialize()

[Tzung-Bi Shih]

In btrtl_initialize(), the `err_free` error path frees `btrtl_dev` and
returns `ERR_PTR(ret)`.  Because earlier calls in btrtl_initialize() set
`ret` to 0 on success, any subsequent failure path that jumps to
`err_free` without setting `ret` to a negative error code causes
btrtl_initialize() to return `ERR_PTR(0)` (`NULL`).

Since the caller btrtl_setup_realtek() only checks `IS_ERR(btrtl_dev)`,
a `NULL` return bypasses the error check and causes a NULL pointer
dereference in btrtl_download_firmware().

- Patch 1 fixes a NULL pointer dereference observed when
  btrtl_vendor_read_reg16() fails.
- Patch 2 fixes a similar issue found by inspection when bt_skb_alloc()
  fails.

Tzung-Bi Shih (2):
  Bluetooth: btrtl: Propagate errors if btrtl_vendor_read_reg16() fails
  Bluetooth: btrtl: Return -ENOMEM if bt_skb_alloc() fails

 drivers/bluetooth/btrtl.c | 9 +++++----
 1 file changed, 5 insertions(+), 4 deletions(-)

-- 
2.56.0.307.g5fe7bd680c-goog

[PATCH 1/2] Bluetooth: btrtl: Propagate errors if btrtl_vendor_read_reg16() fails

[Tzung-Bi Shih]

A NULL pointer dereference is observed in btrtl_download_firmware():

  [ 2592.730992] Bluetooth: btrtl_vendor_read_reg16() hci0: RTL: RTL: Read reg16 failed (-110)
  [ 2592.731005] Bluetooth: btusb_rtl_alloc_devcoredump() hci0: RTL: Failed to generate devcoredump
  [ 2592.731060] BUG: kernel NULL pointer dereference, address: 0000000000000000
  ...
  [ 2592.731906] RIP: 0010:btrtl_download_firmware+0x19/0xb10 [btrtl]
  ...
  [ 2592.732791] Call Trace:
  [ 2592.732883]  <TASK>
  [ 2592.732935]  btrtl_setup_realtek+0x30/0x60 [btrtl]

When btrtl_vendor_read_reg16() for RTL_SEC_PROJ fails in
btrtl_initialize(), the error code is stored in `rc` instead of `ret`,
causing the err_free path to return ERR_PTR(0) (NULL).  As a result,
IS_ERR(btrtl_dev) in btrtl_setup_realtek() evaluates to false and NULL
is passed to btrtl_download_firmware(), leading to a NULL pointer
dereference.

Store the return value in `ret` so the error is properly propagated to
the caller.

Fixes: cd8dbd9ef600 ("Bluetooth: btrtl: Avoid loading the config file on security chips")
Cc: stable@vger.kernel.org
Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
 drivers/bluetooth/btrtl.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/bluetooth/btrtl.c b/drivers/bluetooth/btrtl.c
index d29813331603..d2c2e79800fc 100644
--- a/drivers/bluetooth/btrtl.c
+++ b/drivers/bluetooth/btrtl.c
@@ -1073,7 +1073,6 @@ struct btrtl_device_info *btrtl_initialize(struct hci_dev *hdev,
 	u16 hci_rev, lmp_subver;
 	u8 hci_ver, lmp_ver, chip_type = 0;
 	int ret;
-	int rc;
 	u8 key_id;
 	u8 reg_val[2];
 
@@ -1185,8 +1184,8 @@ struct btrtl_device_info *btrtl_initialize(struct hci_dev *hdev,
 		goto err_free;
 	}
 
-	rc = btrtl_vendor_read_reg16(hdev, RTL_SEC_PROJ, reg_val);
-	if (rc < 0)
+	ret = btrtl_vendor_read_reg16(hdev, RTL_SEC_PROJ, reg_val);
+	if (ret < 0)
 		goto err_free;
 
 	key_id = reg_val[0];
-- 
2.56.0.307.g5fe7bd680c-goog

[PATCH 2/2] Bluetooth: btrtl: Return -ENOMEM if bt_skb_alloc() fails

[Tzung-Bi Shih]

If bt_skb_alloc() fails in btrtl_initialize(), it jumps to `err_free`
without updating `ret` (which is 0 from earlier calls).  As a result, it
returns ERR_PTR(0) (NULL) to its caller, leading to a NULL pointer
dereference in btrtl_download_firmware().

Set `ret` to `-ENOMEM` before jumping to `err_free`.

Fixes: 9a24ce5e29b1 ("Bluetooth: btrtl: Firmware format v2 support")
Cc: stable@vger.kernel.org
Signed-off-by: Tzung-Bi Shih <tzungbi@kernel.org>
---
 drivers/bluetooth/btrtl.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/bluetooth/btrtl.c b/drivers/bluetooth/btrtl.c
index d2c2e79800fc..35a2c9d3380c 100644
--- a/drivers/bluetooth/btrtl.c
+++ b/drivers/bluetooth/btrtl.c
@@ -1143,8 +1143,10 @@ struct btrtl_device_info *btrtl_initialize(struct hci_dev *hdev,
 
 	if (btrtl_dev->drop_fw) {
 		skb = bt_skb_alloc(sizeof(*cmd), GFP_KERNEL);
-		if (!skb)
+		if (!skb) {
+			ret = -ENOMEM;
 			goto err_free;
+		}
 
 		cmd = skb_put(skb, HCI_COMMAND_HDR_SIZE);
 		cmd->opcode = cpu_to_le16(0xfc66);
-- 
2.56.0.307.g5fe7bd680c-goog

RE: Bluetooth: btrtl: Fix NULL return on errors in btrtl_initialize()

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 1460 bytes --]

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/series/1194330/

---Test result---

Test Summary:
CheckPatch                    PASS      1.03 seconds
VerifyFixes                   PASS      0.11 seconds
VerifySignedoff               PASS      0.10 seconds
GitLint                       FAIL      3.35 seconds
SubjectPrefix                 PASS      0.20 seconds
BuildKernel                   PASS      19.01 seconds
CheckAllWarning               PASS      22.73 seconds
CheckSparse                   PASS      25.28 seconds
BuildKernel32                 PASS      19.43 seconds
CheckKernelLLVM               PASS      20.21 seconds
TestRunnerSetup               PASS      452.76 seconds
IncrementalBuild              PASS      20.34 seconds

Details
##############################
Test: GitLint - FAIL
Desc: Run gitlint
Output:
[1/2] Bluetooth: btrtl: Propagate errors if btrtl_vendor_read_reg16() fails

5: B1 Line exceeds max length (94>80): "  [ 2592.730992] Bluetooth: btrtl_vendor_read_reg16() hci0: RTL: RTL: Read reg16 failed (-110)"
6: B1 Line exceeds max length (99>80): "  [ 2592.731005] Bluetooth: btusb_rtl_alloc_devcoredump() hci0: RTL: Failed to generate devcoredump"


https://github.com/bluez/bluetooth-next/pull/1005

---
Regards,
Linux Bluetooth