PATCH] Bluetooth: btnxpuart: Fix out-of-bounds firmware read in nxp_recv_fw_req_v3()

[Neeraj Sanjay Kale]

Hi Maoyi,

Thank you for the fix.

> During the v3 firmware download the controller sends a v3_data_req with a
> 32 bit offset and a 16 bit len. nxp_recv_fw_req_v3() checks only the lower
> bound of the offset and then sends firmware from that offset.
> 
>         nxpdev->fw_dnld_v3_offset = offset - nxpdev->fw_v3_offset_correction;
>         serdev_device_write_buf(nxpdev->serdev, nxpdev->fw->data +
>                                 nxpdev->fw_dnld_v3_offset, len);
> 
> Nothing checks that fw_dnld_v3_offset + len stays within nxpdev->fw->size,
> so a controller that asks for an offset or length past the firmware image makes
> the driver read past the end of nxpdev->fw->data and send that memory
> back over UART.
> 
> nxp_recv_fw_req_v1() already bounds the same write. Add the equivalent
> check to the v3 path, reject the request when it falls outside the firmware
> image, and zero len on the error path so the fw_v3_prev_sent bookkeeping at
> free_skb stays consistent.
> 
> Fixes: 689ca16e5232 ("Bluetooth: NXP: Add protocol support for NXP
> Bluetooth chipsets")
> Suggested-by: Neeraj Kale <neeraj.sanjaykale@xxxxxxx>
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Maoyi Xie <maoyixie.tju@xxxxxxxxx>
> ---
> This is the fix for the out of bounds read I reported on linux-bluetooth. Neeraj
> confirmed the bug and asked me to send it. The Suggested-by is for his
> suggestion to zero len on the reject path, which keeps the fw_v3_prev_sent
> bookkeeping at free_skb consistent.
> 
>  drivers/bluetooth/btnxpuart.c | 6 ++++++
>  1 file changed, 6 insertions(+)
> 
> diff --git a/drivers/bluetooth/btnxpuart.c b/drivers/bluetooth/btnxpuart.c
> index e7036a48ce48..6a1cffe08d5f 100644
> --- a/drivers/bluetooth/btnxpuart.c
> +++ b/drivers/bluetooth/btnxpuart.c
> @@ -1267,6 +1267,12 @@ static int nxp_recv_fw_req_v3(struct hci_dev
> *hdev, struct sk_buff *skb)
>         }
> 
>         nxpdev->fw_dnld_v3_offset = offset - nxpdev->fw_v3_offset_correction;
> +       if (nxpdev->fw_dnld_v3_offset >= nxpdev->fw->size ||
> +           len > nxpdev->fw->size - nxpdev->fw_dnld_v3_offset) {
> +               bt_dev_err(hdev, "FW download out of bounds, ignoring request");
> +               len = 0;
> +               goto free_skb;
> +       }
>         serdev_device_write_buf(nxpdev->serdev, nxpdev->fw->data +
>                                 nxpdev->fw_dnld_v3_offset, len);
> 
> --
> 2.34.1

Reviewed-by: Neeraj Sanjay Kale <neeraj.sanjaykale@xxxxxxx>

Thanks,
Neeraj