* [PATCH] Bluetooth: btnxpuart: Fix out-of-bounds firmware read in nxp_recv_fw_req_v1()
@ 2026-07-05 11:56 Doruk Tan Ozturk
2026-07-05 12:24 ` bluez.test.bot
2026-07-05 20:58 ` [PATCH] " Paul Menzel
0 siblings, 2 replies; 6+ messages in thread
From: Doruk Tan Ozturk @ 2026-07-05 11:56 UTC (permalink / raw)
To: Luiz Augusto von Dentz, Marcel Holtmann, Amitkumar Karwar,
Neeraj Kale
Cc: linux-bluetooth, linux-kernel, stable, Doruk Tan Ozturk
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).
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
Assisted-by: 0sec:claude-opus-4-8
Signed-off-by: Doruk Tan Ozturk <doruk@0sec.ai>
---
drivers/bluetooth/btnxpuart.c | 13 ++++++++++---
1 file changed, 10 insertions(+), 3 deletions(-)
diff --git a/drivers/bluetooth/btnxpuart.c b/drivers/bluetooth/btnxpuart.c
index 6a1cffe08d5f..88d9ebf25a8f 100644
--- a/drivers/bluetooth/btnxpuart.c
+++ b/drivers/bluetooth/btnxpuart.c
@@ -1041,11 +1041,17 @@ 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) {
- if (len == HDR_LEN)
+ if (len == HDR_LEN) {
+ if (nxpdev->fw_dnld_v1_offset >= nxpdev->fw->size ||
+ nxpdev->fw->size - nxpdev->fw_dnld_v1_offset < HDR_LEN) {
+ bt_dev_err(hdev, "FW request offset out of bounds");
+ goto free_skb;
+ }
nxpdev->fw_v1_expected_len = nxp_get_data_len(nxpdev->fw->data +
nxpdev->fw_dnld_v1_offset);
- else
+ } else {
nxpdev->fw_v1_expected_len = HDR_LEN;
+ }
} else if (len == HDR_LEN) {
/* FW download out of sync. Send previous chunk again */
nxpdev->fw_dnld_v1_offset -= nxpdev->fw_v1_sent_bytes;
@@ -1053,7 +1059,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
^ permalink raw reply related [flat|nested] 6+ messages in thread* RE: Bluetooth: btnxpuart: Fix out-of-bounds firmware read in nxp_recv_fw_req_v1() 2026-07-05 11:56 [PATCH] Bluetooth: btnxpuart: Fix out-of-bounds firmware read in nxp_recv_fw_req_v1() Doruk Tan Ozturk @ 2026-07-05 12:24 ` bluez.test.bot 2026-07-05 20:58 ` [PATCH] " Paul Menzel 1 sibling, 0 replies; 6+ messages in thread From: bluez.test.bot @ 2026-07-05 12:24 UTC (permalink / raw) To: linux-bluetooth, doruk [-- Attachment #1: Type: text/plain, Size: 1181 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=1121769 ---Test result--- Test Summary: CheckPatch PASS 0.56 seconds VerifyFixes PASS 0.44 seconds VerifySignedoff PASS 0.07 seconds GitLint PASS 0.20 seconds SubjectPrefix PASS 0.06 seconds BuildKernel PASS 27.62 seconds CheckAllWarning PASS 29.78 seconds CheckSparse PASS 28.33 seconds BuildKernel32 PASS 26.47 seconds CheckKernelLLVM SKIP 0.00 seconds TestRunnerSetup PASS 509.22 seconds IncrementalBuild PASS 25.23 seconds Details ############################## Test: CheckKernelLLVM - SKIP Desc: Build kernel with LLVM + context analysis Output: Clang not found https://github.com/bluez/bluetooth-next/pull/399 --- Regards, Linux Bluetooth ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Bluetooth: btnxpuart: Fix out-of-bounds firmware read in nxp_recv_fw_req_v1() 2026-07-05 11:56 [PATCH] Bluetooth: btnxpuart: Fix out-of-bounds firmware read in nxp_recv_fw_req_v1() Doruk Tan Ozturk 2026-07-05 12:24 ` bluez.test.bot @ 2026-07-05 20:58 ` Paul Menzel 2026-07-06 4:20 ` Neeraj Sanjay Kale 1 sibling, 1 reply; 6+ messages in thread From: Paul Menzel @ 2026-07-05 20:58 UTC (permalink / raw) To: Doruk Tan Ozturk Cc: Luiz Augusto von Dentz, Marcel Holtmann, Amitkumar Karwar, Neeraj Kale, linux-bluetooth, linux-kernel, stable Dear Doruk, Thank you for the patch. Am 05.07.26 um 13:56 schrieb Doruk Tan Ozturk: > 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). > > 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 > Assisted-by: 0sec:claude-opus-4-8 > Signed-off-by: Doruk Tan Ozturk <doruk@0sec.ai> > --- > drivers/bluetooth/btnxpuart.c | 13 ++++++++++--- > 1 file changed, 10 insertions(+), 3 deletions(-) > > diff --git a/drivers/bluetooth/btnxpuart.c b/drivers/bluetooth/btnxpuart.c > index 6a1cffe08d5f..88d9ebf25a8f 100644 > --- a/drivers/bluetooth/btnxpuart.c > +++ b/drivers/bluetooth/btnxpuart.c > @@ -1041,11 +1041,17 @@ 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) { > - if (len == HDR_LEN) > + if (len == HDR_LEN) { > + if (nxpdev->fw_dnld_v1_offset >= nxpdev->fw->size || > + nxpdev->fw->size - nxpdev->fw_dnld_v1_offset < HDR_LEN) { > + bt_dev_err(hdev, "FW request offset out of bounds"); Would it make sense to log all the values, as I’d think, such an issue might be hard to reproduce and gathering the values miht be difficult? > + goto free_skb; > + } > nxpdev->fw_v1_expected_len = nxp_get_data_len(nxpdev->fw->data + > nxpdev->fw_dnld_v1_offset); > - else > + } else { > nxpdev->fw_v1_expected_len = HDR_LEN; > + } > } else if (len == HDR_LEN) { > /* FW download out of sync. Send previous chunk again */ > nxpdev->fw_dnld_v1_offset -= nxpdev->fw_v1_sent_bytes; > @@ -1053,7 +1059,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; Kind regards, Paul ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Bluetooth: btnxpuart: Fix out-of-bounds firmware read in nxp_recv_fw_req_v1() 2026-07-05 20:58 ` [PATCH] " Paul Menzel @ 2026-07-06 4:20 ` Neeraj Sanjay Kale 2026-07-09 21:05 ` Doruk (0sec) 0 siblings, 1 reply; 6+ messages in thread From: Neeraj Sanjay Kale @ 2026-07-06 4:20 UTC (permalink / raw) To: Paul Menzel, Doruk Tan Ozturk Cc: Luiz Augusto von Dentz, Marcel Holtmann, Amitkumar Karwar, linux-bluetooth@vger.kernel.org, linux-kernel@vger.kernel.org, stable@vger.kernel.org Hi Doruk, Thank you for submitting this patch. However, a similar patch is already in review and approved by me: https://patchwork.kernel.org/project/bluetooth/patch/tencent_F2E2AF1B6F510577B10C6897ED768BBBAF07@qq.com/ It's awaiting Luiz's review and/or merge. Hi Luiz, Can you please review the patch mentioned in the URL above, from Zhao Dongdong? I have answered your review comment. Thank you for your time and review. Thanks, Neeraj > Dear Doruk, > > > Thank you for the patch. > > Am 05.07.26 um 13:56 schrieb Doruk Tan Ozturk: > > 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). > > > > This was found by 0sec automated security-research tooling > > > (https://0sec.a/ > i%2F&data=05%7C02%7Cneeraj.sanjaykale%40nxp.com%7Cc82fdb86e33f476 > 570ed08dedad83110%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7 > C639188819230990815%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiO > nRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyf > Q%3D%3D%7C0%7C%7C%7C&sdata=z6YC4OGfeSW45U2PbFFlFz13DG3%2FSr > qYeFKMSNTiMBI%3D&reserved=0). > > > > Fixes: 689ca16e5232 ("Bluetooth: NXP: Add protocol support for NXP > > Bluetooth chipsets") > > Cc: stable@vger.kernel.org > > Assisted-by: 0sec:claude-opus-4-8 > > Signed-off-by: Doruk Tan Ozturk <doruk@0sec.ai> > > --- > > drivers/bluetooth/btnxpuart.c | 13 ++++++++++--- > > 1 file changed, 10 insertions(+), 3 deletions(-) > > > > diff --git a/drivers/bluetooth/btnxpuart.c > > b/drivers/bluetooth/btnxpuart.c index 6a1cffe08d5f..88d9ebf25a8f > > 100644 > > --- a/drivers/bluetooth/btnxpuart.c > > +++ b/drivers/bluetooth/btnxpuart.c > > @@ -1041,11 +1041,17 @@ 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) { > > - if (len == HDR_LEN) > > + if (len == HDR_LEN) { > > + if (nxpdev->fw_dnld_v1_offset >= nxpdev->fw->size || > > + nxpdev->fw->size - nxpdev->fw_dnld_v1_offset < > HDR_LEN) { > > + bt_dev_err(hdev, "FW request > > + offset out of bounds"); > > Would it make sense to log all the values, as I'd think, such an issue might be > hard to reproduce and gathering the values miht be difficult? > > > + goto free_skb; > > + } > > nxpdev->fw_v1_expected_len = nxp_get_data_len(nxpdev- > >fw->data + > > nxpdev->fw_dnld_v1_offset); > > - else > > + } else { > > nxpdev->fw_v1_expected_len = HDR_LEN; > > + } > > } else if (len == HDR_LEN) { > > /* FW download out of sync. Send previous chunk again */ > > nxpdev->fw_dnld_v1_offset -= > > nxpdev->fw_v1_sent_bytes; @@ -1053,7 +1059,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; > > > Kind regards, > > Paul ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Bluetooth: btnxpuart: Fix out-of-bounds firmware read in nxp_recv_fw_req_v1() 2026-07-06 4:20 ` Neeraj Sanjay Kale @ 2026-07-09 21:05 ` Doruk (0sec) 2026-07-10 4:20 ` Neeraj Sanjay Kale 0 siblings, 1 reply; 6+ messages in thread From: Doruk (0sec) @ 2026-07-09 21:05 UTC (permalink / raw) To: Neeraj Sanjay Kale Cc: Paul Menzel, Luiz Augusto von Dentz, Marcel Holtmann, Amitkumar Karwar, linux-bluetooth@vger.kernel.org, linux-kernel@vger.kernel.org, stable@vger.kernel.org Hi Neeraj, Thank you, and sorry if I'm missing sth! That patchwork link points to Zhao Dongdong's "Fix use-after-free in probe error path" patch, which addresses the probe-path use-after-free rather than the firmware-download read. On the firmware-read side: commit 25c286d75821 ("Bluetooth: btnxpuart: Fix out-of-bounds firmware read in nxp_recv_fw_req_v3()") bounded the v3 handler. My patch targets the v1 handler, nxp_recv_fw_req_v1(), where (at least in today's bluetooth-next) the header length is still read at nxp_get_data_len(nxpdev->fw->data + nxpdev->fw_dnld_v1_offset) with no bound on fw_dnld_v1_offset, and the payload write still uses the non-overflow-safe "fw_dnld_v1_offset + len <= fw->size" form (fw_dnld_v1_offset is u32, so the sum can wrap). If it's still open, I'm happy to send a v2 that also logs the offset/size values, as Paul suggested. Either way, thanks very much for taking the time, and I'm happy to defer to whatever you have queued. Best, Doruk On Mon, 6 Jul 2026 at 06:20, Neeraj Sanjay Kale <neeraj.sanjaykale@nxp.com> wrote: > > Hi Doruk, > > Thank you for submitting this patch. > > However, a similar patch is already in review and approved by me: > https://patchwork.kernel.org/project/bluetooth/patch/tencent_F2E2AF1B6F510577B10C6897ED768BBBAF07@qq.com/ > It's awaiting Luiz's review and/or merge. > > > Hi Luiz, > > Can you please review the patch mentioned in the URL above, from Zhao Dongdong? I have answered your review comment. > Thank you for your time and review. > > Thanks, > Neeraj > > > > Dear Doruk, > > > > > > Thank you for the patch. > > > > Am 05.07.26 um 13:56 schrieb Doruk Tan Ozturk: > > > 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). > > > > > > This was found by 0sec automated security-research tooling > > > > > (https://0sec.a/ > > i%2F&data=05%7C02%7Cneeraj.sanjaykale%40nxp.com%7Cc82fdb86e33f476 > > 570ed08dedad83110%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7 > > C639188819230990815%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiO > > nRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyf > > Q%3D%3D%7C0%7C%7C%7C&sdata=z6YC4OGfeSW45U2PbFFlFz13DG3%2FSr > > qYeFKMSNTiMBI%3D&reserved=0). > > > > > > Fixes: 689ca16e5232 ("Bluetooth: NXP: Add protocol support for NXP > > > Bluetooth chipsets") > > > Cc: stable@vger.kernel.org > > > Assisted-by: 0sec:claude-opus-4-8 > > > Signed-off-by: Doruk Tan Ozturk <doruk@0sec.ai> > > > --- > > > drivers/bluetooth/btnxpuart.c | 13 ++++++++++--- > > > 1 file changed, 10 insertions(+), 3 deletions(-) > > > > > > diff --git a/drivers/bluetooth/btnxpuart.c > > > b/drivers/bluetooth/btnxpuart.c index 6a1cffe08d5f..88d9ebf25a8f > > > 100644 > > > --- a/drivers/bluetooth/btnxpuart.c > > > +++ b/drivers/bluetooth/btnxpuart.c > > > @@ -1041,11 +1041,17 @@ 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) { > > > - if (len == HDR_LEN) > > > + if (len == HDR_LEN) { > > > + if (nxpdev->fw_dnld_v1_offset >= nxpdev->fw->size || > > > + nxpdev->fw->size - nxpdev->fw_dnld_v1_offset < > > HDR_LEN) { > > > + bt_dev_err(hdev, "FW request > > > + offset out of bounds"); > > > > Would it make sense to log all the values, as I'd think, such an issue might be > > hard to reproduce and gathering the values miht be difficult? > > > > > + goto free_skb; > > > + } > > > nxpdev->fw_v1_expected_len = nxp_get_data_len(nxpdev- > > >fw->data + > > > nxpdev->fw_dnld_v1_offset); > > > - else > > > + } else { > > > nxpdev->fw_v1_expected_len = HDR_LEN; > > > + } > > > } else if (len == HDR_LEN) { > > > /* FW download out of sync. Send previous chunk again */ > > > nxpdev->fw_dnld_v1_offset -= > > > nxpdev->fw_v1_sent_bytes; @@ -1053,7 +1059,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; > > > > > > Kind regards, > > > > Paul ^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH] Bluetooth: btnxpuart: Fix out-of-bounds firmware read in nxp_recv_fw_req_v1() 2026-07-09 21:05 ` Doruk (0sec) @ 2026-07-10 4:20 ` Neeraj Sanjay Kale 0 siblings, 0 replies; 6+ messages in thread From: Neeraj Sanjay Kale @ 2026-07-10 4:20 UTC (permalink / raw) To: Doruk (0sec) Cc: Paul Menzel, Luiz Augusto von Dentz, Marcel Holtmann, Amitkumar Karwar, linux-bluetooth@vger.kernel.org, linux-kernel@vger.kernel.org, stable@vger.kernel.org Hi Doruk, Apologies for the confusion. I was referring to a different patch but looking more closely I realised both are different. Apologies again. Your patch tries to prevent an unbounded access in nxp_get_data_len() which seems genuine in a corrupt FW file or malicious controller. Please do send a V2 patch as Paul requested with all offset, expected_len and size values logged, along with the reviewed-by tag. Thanks, Neeraj Kale Reviewed-by: Neeraj Sanjay Kale <neeraj.sanjaykale@nxp.com> > Hi Neeraj, > > Thank you, and sorry if I'm missing sth! > > That patchwork link points to Zhao > Dongdong's "Fix use-after-free in probe error path" patch, which addresses > the probe-path use-after-free rather than the firmware-download read. > > On the firmware-read side: commit 25c286d75821 ("Bluetooth: btnxpuart: Fix > out-of-bounds firmware read in nxp_recv_fw_req_v3()") bounded the v3 > handler. > My patch targets the v1 handler, nxp_recv_fw_req_v1(), where (at least in > today's bluetooth-next) the header length is still read at > nxp_get_data_len(nxpdev->fw->data + nxpdev->fw_dnld_v1_offset) > > with no bound on fw_dnld_v1_offset, and the payload write still uses the > non-overflow-safe "fw_dnld_v1_offset + len <= fw->size" form > (fw_dnld_v1_offset is u32, so the sum can wrap). > > If it's still open, I'm happy to send a v2 that also logs the offset/size values, as > Paul suggested. Either way, thanks very much for taking the time, and I'm > happy to defer to whatever you have queued. > > Best, > Doruk > > > > On Mon, 6 Jul 2026 at 06:20, Neeraj Sanjay Kale > <neeraj.sanjaykale@nxp.com> wrote: > > > > Hi Doruk, > > > > Thank you for submitting this patch. > > > > However, a similar patch is already in review and approved by me: > > https://patc/ > > > hwork.kernel.org%2Fproject%2Fbluetooth%2Fpatch%2Ftencent_F2E2AF1B6F5 > 10 > > > 577B10C6897ED768BBBAF07%40qq.com%2F&data=05%7C02%7Cneeraj.sanja > ykale%4 > > > 0nxp.com%7C667652efd25047817cd808deddfddf56%7C686ea1d3bc2b4c6fa9 > 2cd99c > > > 5c301635%7C0%7C0%7C639192279635192171%7CUnknown%7CTWFpbGZsb > 3d8eyJFbXB0 > > > eU1hcGkiOnRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpb > CIsIl > > > dUIjoyfQ%3D%3D%7C0%7C%7C%7C&sdata=QYHW%2B%2BB2KeS1FlSJb36v4 > NHS1%2BLa8W > > q0oQscOroZra0%3D&reserved=0 It's awaiting Luiz's review and/or merge. > > > > > > Hi Luiz, > > > > Can you please review the patch mentioned in the URL above, from Zhao > Dongdong? I have answered your review comment. > > Thank you for your time and review. > > > > Thanks, > > Neeraj > > > > > > > Dear Doruk, > > > > > > > > > Thank you for the patch. > > > > > > Am 05.07.26 um 13:56 schrieb Doruk Tan Ozturk: > > > > 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). > > > > > > > > This was found by 0sec automated security-research tooling > > > > > > > (https://0.0.0.0/ > > > > sec.a%2F&data=05%7C02%7Cneeraj.sanjaykale%40nxp.com%7C667652efd25 > 047 > > > > 817cd808deddfddf56%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7 > C6391 > > > > 92279635231913%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiOnRyd > WUsIlYiO > > > > iIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyfQ%3D%3D%7C > 0% > > > > 7C%7C%7C&sdata=nLlRKyBusMXrHIK5NjCqZS7C0T6FTLfszBmagPI%2BDqQ%3 > D&rese > > > rved=0 > > > > i%2F&data=05%7C02%7Cneeraj.sanjaykale%40nxp.com%7Cc82fdb86e33f476 > > > > 570ed08dedad83110%7C686ea1d3bc2b4c6fa92cd99c5c301635%7C0%7C0%7 > > > > C639188819230990815%7CUnknown%7CTWFpbGZsb3d8eyJFbXB0eU1hcGkiO > > > > nRydWUsIlYiOiIwLjAuMDAwMCIsIlAiOiJXaW4zMiIsIkFOIjoiTWFpbCIsIldUIjoyf > > > > Q%3D%3D%7C0%7C%7C%7C&sdata=z6YC4OGfeSW45U2PbFFlFz13DG3%2FSr > > > qYeFKMSNTiMBI%3D&reserved=0). > > > > > > > > Fixes: 689ca16e5232 ("Bluetooth: NXP: Add protocol support for NXP > > > > Bluetooth chipsets") > > > > Cc: stable@vger.kernel.org > > > > Assisted-by: 0sec:claude-opus-4-8 > > > > Signed-off-by: Doruk Tan Ozturk <doruk@0sec.ai> > > > > --- > > > > drivers/bluetooth/btnxpuart.c | 13 ++++++++++--- > > > > 1 file changed, 10 insertions(+), 3 deletions(-) > > > > > > > > diff --git a/drivers/bluetooth/btnxpuart.c > > > > b/drivers/bluetooth/btnxpuart.c index 6a1cffe08d5f..88d9ebf25a8f > > > > 100644 > > > > --- a/drivers/bluetooth/btnxpuart.c > > > > +++ b/drivers/bluetooth/btnxpuart.c > > > > @@ -1041,11 +1041,17 @@ 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) { > > > > - if (len == HDR_LEN) > > > > + if (len == HDR_LEN) { > > > > + if (nxpdev->fw_dnld_v1_offset >= nxpdev->fw->size || > > > > + nxpdev->fw->size - > > > > + nxpdev->fw_dnld_v1_offset < > > > HDR_LEN) { > > > > + bt_dev_err(hdev, "FW request > > > > + offset out of bounds"); > > > > > > Would it make sense to log all the values, as I'd think, such an > > > issue might be hard to reproduce and gathering the values miht be > difficult? > > > > > > > + goto free_skb; > > > > + } > > > > nxpdev->fw_v1_expected_len = > > > > nxp_get_data_len(nxpdev- > > > >fw->data + > > > > nxpdev->fw_dnld_v1_offset); > > > > - else > > > > + } else { > > > > nxpdev->fw_v1_expected_len = > > > > HDR_LEN; > > > > + } > > > > } else if (len == HDR_LEN) { > > > > /* FW download out of sync. Send previous chunk again */ > > > > nxpdev->fw_dnld_v1_offset -= > > > > nxpdev->fw_v1_sent_bytes; @@ -1053,7 +1059,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; > > > > > > > > > Kind regards, > > > > > > Paul ^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2026-07-10 4:20 UTC | newest] Thread overview: 6+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2026-07-05 11:56 [PATCH] Bluetooth: btnxpuart: Fix out-of-bounds firmware read in nxp_recv_fw_req_v1() Doruk Tan Ozturk 2026-07-05 12:24 ` bluez.test.bot 2026-07-05 20:58 ` [PATCH] " Paul Menzel 2026-07-06 4:20 ` Neeraj Sanjay Kale 2026-07-09 21:05 ` Doruk (0sec) 2026-07-10 4:20 ` Neeraj Sanjay Kale
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox