[PATCH] Bluetooth: btnxpuart: Fix out-of-bounds firmware read in nxp_recv_fw_req_v3()

[PATCH] Bluetooth: btnxpuart: Fix out-of-bounds firmware read in nxp_recv_fw_req_v3()

[Maoyi Xie]

During the v3 firmware download the controller sends a v3_data_req with a
32 bit offset and a 16 bit len. nxp_recv_fw_req_v3() checks only the lower
bound of the offset and then sends firmware from that offset.

	nxpdev->fw_dnld_v3_offset = offset - nxpdev->fw_v3_offset_correction;
	serdev_device_write_buf(nxpdev->serdev, nxpdev->fw->data +
				nxpdev->fw_dnld_v3_offset, len);

Nothing checks that fw_dnld_v3_offset + len stays within nxpdev->fw->size, so a
controller that asks for an offset or length past the firmware image makes the
driver read past the end of nxpdev->fw->data and send that memory back over
UART.

nxp_recv_fw_req_v1() already bounds the same write. Add the equivalent check to
the v3 path, reject the request when it falls outside the firmware image, and
zero len on the error path so the fw_v3_prev_sent bookkeeping at free_skb stays
consistent.

Fixes: 689ca16e5232 ("Bluetooth: NXP: Add protocol support for NXP Bluetooth chipsets")
Suggested-by: Neeraj Kale <neeraj.sanjaykale@nxp.com>
Cc: stable@vger.kernel.org
Signed-off-by: Maoyi Xie <maoyixie.tju@gmail.com>
---
This is the fix for the out of bounds read I reported on linux-bluetooth. Neeraj
confirmed the bug and asked me to send it. The Suggested-by is for his suggestion
to zero len on the reject path, which keeps the fw_v3_prev_sent bookkeeping at
free_skb consistent.

 drivers/bluetooth/btnxpuart.c | 6 ++++++
 1 file changed, 6 insertions(+)

diff --git a/drivers/bluetooth/btnxpuart.c b/drivers/bluetooth/btnxpuart.c
index e7036a48ce48..6a1cffe08d5f 100644
--- a/drivers/bluetooth/btnxpuart.c
+++ b/drivers/bluetooth/btnxpuart.c
@@ -1267,6 +1267,12 @@ static int nxp_recv_fw_req_v3(struct hci_dev *hdev, struct sk_buff *skb)
 	}
 
 	nxpdev->fw_dnld_v3_offset = offset - nxpdev->fw_v3_offset_correction;
+	if (nxpdev->fw_dnld_v3_offset >= nxpdev->fw->size ||
+	    len > nxpdev->fw->size - nxpdev->fw_dnld_v3_offset) {
+		bt_dev_err(hdev, "FW download out of bounds, ignoring request");
+		len = 0;
+		goto free_skb;
+	}
 	serdev_device_write_buf(nxpdev->serdev, nxpdev->fw->data +
 				nxpdev->fw_dnld_v3_offset, len);
 
-- 
2.34.1

RE: Bluetooth: btnxpuart: Fix out-of-bounds firmware read in nxp_recv_fw_req_v3()

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 2563 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/project/bluetooth/list/?series=1112441

---Test result---

Test Summary:
CheckPatch                    FAIL      0.44 seconds
VerifyFixes                   PASS      0.08 seconds
VerifySignedoff               PASS      0.08 seconds
GitLint                       FAIL      0.21 seconds
SubjectPrefix                 PASS      0.07 seconds
BuildKernel                   PASS      16.91 seconds
CheckAllWarning               PASS      20.08 seconds
CheckSparse                   PASS      18.00 seconds
BuildKernel32                 PASS      16.75 seconds
CheckKernelLLVM               SKIP      0.00 seconds
TestRunnerSetup               PASS      329.06 seconds
IncrementalBuild              PASS      16.02 seconds

Details
##############################
Test: CheckPatch - FAIL
Desc: Run checkpatch.pl script
Output:
Bluetooth: btnxpuart: Fix out-of-bounds firmware read in nxp_recv_fw_req_v3()
WARNING: Prefer a maximum 75 chars per line (possible unwrapped commit description?)
#108: 
Nothing checks that fw_dnld_v3_offset + len stays within nxpdev->fw->size, so a

total: 0 errors, 1 warnings, 12 lines checked

NOTE: For some of the reported defects, checkpatch may be able to
      mechanically convert to the typical style using --fix or --fix-inplace.

/github/workspace/src/patch/14631861.patch has style problems, please review.

NOTE: Ignored message types: UNKNOWN_COMMIT_ID

NOTE: If any of the errors are false positives, please report
      them to the maintainer, see CHECKPATCH in MAINTAINERS.


##############################
Test: GitLint - FAIL
Desc: Run gitlint
Output:
Bluetooth: btnxpuart: Fix out-of-bounds firmware read in nxp_recv_fw_req_v3()

7: B3 Line contains hard tab characters (\t): "	nxpdev->fw_dnld_v3_offset = offset - nxpdev->fw_v3_offset_correction;"
8: B3 Line contains hard tab characters (\t): "	serdev_device_write_buf(nxpdev->serdev, nxpdev->fw->data +"
9: B3 Line contains hard tab characters (\t): "				nxpdev->fw_dnld_v3_offset, len);"
26: B1 Line exceeds max length (81>80): "confirmed the bug and asked me to send it. The Suggested-by is for his suggestion"
##############################
Test: CheckKernelLLVM - SKIP
Desc: Build kernel with LLVM + context analysis
Output:
Clang not found


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

---
Regards,
Linux Bluetooth