[PATCH v3] Bluetooth: btnxpuart: Fix out-of-bounds read in nxp_recv_fw_req_v1()

Doruk Tan Ozturk posted 1 patch 3 days, 2 hours ago
drivers/bluetooth/btnxpuart.c | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
[PATCH v3] Bluetooth: btnxpuart: Fix out-of-bounds read in nxp_recv_fw_req_v1()
Posted by Doruk Tan Ozturk 3 days, 2 hours ago
Commit 25c286d75821 ("Bluetooth: btnxpuart: Fix out-of-bounds firmware
read in nxp_recv_fw_req_v3()") bounded the v3 firmware download offset but
left an unbounded read in the v1 handler.

nxp_recv_fw_req_v1() advances a device-driven download offset
(fw_dnld_v1_offset) by fw_v1_sent_bytes on every request, and that
bookkeeping runs even when the payload write is skipped, so the offset can
walk past nxpdev->fw->size. When the controller then requests a header
(len == HDR_LEN), the driver reads the 16-byte bootloader header at

  nxp_get_data_len(nxpdev->fw->data + nxpdev->fw_dnld_v1_offset)

with no bound on the offset, reading past the end of the firmware image.
A malicious or malfunctioning NXP UART controller can drive this to read
out-of-bounds kernel memory during firmware download.

Bound the offset before the header read, and convert the payload write
guard to the overflow-safe form used by the v3 path (fw_dnld_v1_offset is
u32, so fw_dnld_v1_offset + len can wrap). Log the offset and firmware
size at the reject site to aid debugging of a real device.

This was found by 0sec automated security-research tooling
(https://0sec.ai).

Fixes: 689ca16e5232 ("Bluetooth: NXP: Add protocol support for NXP Bluetooth chipsets")
Cc: stable@vger.kernel.org
Reviewed-by: Neeraj Sanjay Kale <neeraj.sanjaykale@nxp.com>
Assisted-by: 0sec:claude-opus-4-8
Signed-off-by: Doruk Tan Ozturk <doruk@0sec.ai>
---
v3: reformat the reject path to stay within the 80-column limit (hoist
    offset/size to locals, fold the header-request and bounds tests
    into one guard); no functional change vs v2.  Drop expected_len
    from the log line -- it is always HDR_LEN at the reject site.
    Shorten the subject to 80 columns.  Carries the Reviewed-by,
    the logic is unchanged.

v2: log offset/fw size at the reject site (requested by Neeraj Kale
    and Paul Menzel).

v1: https://lore.kernel.org/linux-bluetooth/20260705115650.81724-1-doruk@0sec.ai/
 drivers/bluetooth/btnxpuart.c | 13 ++++++++++++-
 1 file changed, 12 insertions(+), 1 deletion(-)

diff --git a/drivers/bluetooth/btnxpuart.c b/drivers/bluetooth/btnxpuart.c
index 0bb300eef157..423a20f78baa 100644
--- a/drivers/bluetooth/btnxpuart.c
+++ b/drivers/bluetooth/btnxpuart.c
@@ -1041,6 +1041,16 @@ static int nxp_recv_fw_req_v1(struct hci_dev *hdev, struct sk_buff *skb)
 		 * and we need to re-send the previous header again.
 		 */
 		if (len == nxpdev->fw_v1_expected_len) {
+			u32 off = nxpdev->fw_dnld_v1_offset;
+			size_t sz = nxpdev->fw->size;
+
+			if (len == HDR_LEN &&
+			    (off >= sz || sz - off < HDR_LEN)) {
+				bt_dev_err(hdev,
+					   "FW req OOB: offset %u, size %zu",
+					   off, sz);
+				goto free_skb;
+			}
 			if (len == HDR_LEN)
 				nxpdev->fw_v1_expected_len = nxp_get_data_len(nxpdev->fw->data +
 									nxpdev->fw_dnld_v1_offset);
@@ -1053,7 +1063,8 @@ static int nxp_recv_fw_req_v1(struct hci_dev *hdev, struct sk_buff *skb)
 		}
 	}
 
-	if (nxpdev->fw_dnld_v1_offset + len <= nxpdev->fw->size)
+	if (nxpdev->fw_dnld_v1_offset < nxpdev->fw->size &&
+	    len <= nxpdev->fw->size - nxpdev->fw_dnld_v1_offset)
 		serdev_device_write_buf(nxpdev->serdev, nxpdev->fw->data +
 					nxpdev->fw_dnld_v1_offset, len);
 	nxpdev->fw_v1_sent_bytes = len;
-- 
2.43.0