mirror of https://lore.kernel.org/lkml/
 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 20:58 ` Paul Menzel
  0 siblings, 1 reply; 5+ 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	[flat|nested] 5+ 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 20:58 ` Paul Menzel
  2026-07-06  4:20   ` Neeraj Sanjay Kale
  0 siblings, 1 reply; 5+ 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] 5+ messages in thread

* Re: [PATCH] Bluetooth: btnxpuart: Fix out-of-bounds firmware read in nxp_recv_fw_req_v1()
  2026-07-05 20:58 ` Paul Menzel
@ 2026-07-06  4:20   ` Neeraj Sanjay Kale
  2026-07-09 21:05     ` Doruk (0sec)
  0 siblings, 1 reply; 5+ 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, linux-kernel, stable

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] 5+ 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; 5+ 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, linux-kernel, stable

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] 5+ 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; 5+ 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, linux-kernel, stable

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] 5+ messages in thread

end of thread, other threads:[~2026-07-10  4:20 UTC | newest]

Thread overview: 5+ 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 20:58 ` 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