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

Hi Doruk,

On Mon, Jul 20, 2026 at 3:13 PM Doruk Tan Ozturk <[email protected]> wrote:
>
> 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, expected length
> 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: [email protected]
> Reviewed-by: Neeraj Sanjay Kale <[email protected]>
> Assisted-by: 0sec:claude-opus-4-8
> Signed-off-by: Doruk Tan Ozturk <[email protected]>
> ---
> v2: log offset/expected_len/fw size at the reject site (requested by
>     Neeraj Kale and Paul Menzel).  No functional change to the bound
>     itself vs v1.
>
> v1: https://lore.kernel.org/linux-bluetooth/[email protected]/
>  drivers/bluetooth/btnxpuart.c | 15 ++++++++++++---
>  1 file changed, 12 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/bluetooth/btnxpuart.c b/drivers/bluetooth/btnxpuart.c
> index 0bb300eef157..6d62d07a542b 100644
> --- a/drivers/bluetooth/btnxpuart.c
> +++ b/drivers/bluetooth/btnxpuart.c
> @@ -1041,11 +1041,19 @@ 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 out of bounds: offset %u, expected_len %u, fw size %zu",

We still have the 80 columns limit in Bluetooth.

> +                                                  nxpdev->fw_dnld_v1_offset, len,
> +                                                  nxpdev->fw->size);
> +                                       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 +1061,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
>


-- 
Luiz Augusto von Dentz