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

[Pauli Virtanen]

Hi,

ma, 2026-06-29 kello 23:28 -0700, Weiming Shi kirjoitti:
> 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;
> +	}

Was this tested on real hardware?

With an out-of-bounds access fix like the above in btmtk some weeks
ago, it turned out HW actually was sending truncated data, and it was
found only later in production use that the driver was broken by the
fix.

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

-- 
Pauli Virtanen