Linux bluetooth development
 help / color / mirror / Atom feed
* [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