Re: [PATCH] Bluetooth: bpa10x: check response length in bpa10x_setup()

[Luiz Augusto von Dentz]

Hi Weiming,

On Tue, Jun 30, 2026 at 2:28 AM Weiming Shi <bestswngs@xxxxxxxxx> wrote:
>
> bpa10x_setup() sends the vendor command 0xfc0e and passes the response
> to bt_dev_info() and hci_set_fw_info() as a "%s" string starting at
> skb->data + 1, without checking the length:
>
>         bt_dev_info(hdev, "%s", (char *)(skb->data + 1));
>         hci_set_fw_info(hdev, "%s", skb->data + 1);
>
> A device that returns a one-byte response (status only) leaves
> skb->data + 1 past the end of the data, and the %s walk reads adjacent
> slab memory until it meets a NUL. The same happens when the payload is
> not NUL-terminated within skb->len. The out-of-bounds bytes end up in
> the kernel log and the firmware-info debugfs file.
>
> Bail out if the response is too short or not NUL-terminated.
>
> Fixes: ddd68ec8f484 ("Bluetooth: bpa10x: Read revision information in setup stage")
> Reported-by: Xiang Mei <xmei5@xxxxxxx>
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Weiming Shi <bestswngs@xxxxxxxxx>
> ---
>  drivers/bluetooth/bpa10x.c | 6 ++++++
>  1 file changed, 6 insertions(+)
>
> diff --git a/drivers/bluetooth/bpa10x.c b/drivers/bluetooth/bpa10x.c
> index 2ae38a321c4b..07c6ed6f18ac 100644
> --- a/drivers/bluetooth/bpa10x.c
> +++ b/drivers/bluetooth/bpa10x.c
> @@ -255,6 +255,12 @@ static int bpa10x_setup(struct hci_dev *hdev)
>         if (IS_ERR(skb))
>                 return PTR_ERR(skb);
>
> +       /* Reject a short or non-terminated response before the %s reads. */
> +       if (skb->len < 2 || skb->data[skb->len - 1] != '\0') {
> +               kfree_skb(skb);
> +               return -EILSEQ;
> +       }
> +

Looks valid to me. We shouldn't really depend on the string being
NULL-terminated, as it could be truncated for some reason. Need to
check if %.*s is safe though and if that would work with the likes of
hci_set_fw_info:

https://sashiko.dev/#/patchset/20260630062759.2769059-3-bestswngs%40gmail.com

>         bt_dev_info(hdev, "%s", (char *)(skb->data + 1));
>
>         hci_set_fw_info(hdev, "%s", skb->data + 1);
> --
> 2.43.0
>


-- 
Luiz Augusto von Dentz