Re: [PATCH bluetooth] Bluetooth: qca: fix NVM tag length underflow in TLV parser

[Johan Hovold <johan@kernel.org>]

On Sat, Jul 04, 2026 at 04:10:30PM -0700, Xiang Mei wrote:
> In the TLV_TYPE_NVM branch of qca_tlv_check_data() the tag loop bound is
> "while (idx < length - sizeof(struct tlv_type_nvm))". "length" is a signed
> int from the firmware TLV header and sizeof(struct tlv_type_nvm) is a
> size_t (12), so "length" is converted to size_t and any firmware-supplied
> "length" < 12 makes the subtraction wrap to a huge value.

Yeah, there should have been a lower bound check on length before the
loop.

> The loop body
> then reads a 12-byte struct tlv_type_nvm past the end of the short
> vmalloc'd firmware buffer (and the EDL_TAG_ID_* handlers can write past it).

No, only four bytes are read before the loop exits without any further
consequences due to the following check:

	 if (length < idx + sizeof(struct tlv_type_nvm) + tag_len)
	 	return -EINVAL;

which is false if length < 12.

> Rewrite the bound as "idx + sizeof(struct tlv_type_nvm) <= length"; both
> operands are non-negative, so it no longer underflows and a "length" too
> small for one record correctly skips the loop.

This works too.

>   BUG: KASAN: vmalloc-out-of-bounds in qca_download_firmware.isra.0 (drivers/bluetooth/btqca.c:421)
>   Read of size 2 at addr ffffc900000e5004 by task kworker/u9:0/52
>   Workqueue: hci0 hci_power_on
>   Call Trace:
>    ...
>    kasan_report (mm/kasan/report.c:595)
>    qca_download_firmware.isra.0 (drivers/bluetooth/btqca.c:421 drivers/bluetooth/btqca.c:617)
>    qca_uart_setup (drivers/bluetooth/btqca.c:948)
>    qca_setup (drivers/bluetooth/hci_qca.c:2029)
>    hci_uart_setup (drivers/bluetooth/hci_ldisc.c:438)
>    hci_dev_open_sync (net/bluetooth/hci_sync.c:5227)
>    hci_power_on (net/bluetooth/hci_core.c:920)
>    process_one_work (kernel/workqueue.c:3322)
>    worker_thread (kernel/workqueue.c:3486)
>    kthread (kernel/kthread.c:436)
>    ret_from_fork (arch/x86/kernel/process.c:158)
>    ret_from_fork_asm (arch/x86/entry/entry_64.S:245)
> 
> Fixes: 2e4edfa1e2bd ("Bluetooth: qca: add missing firmware sanity checks")
> Reported-by: Weiming Shi <bestswngs@gmail.com>
> Assisted-by: Claude:claude-opus-4-8
> Signed-off-by: Xiang Mei <xmei5@asu.edu>
> ---
>  drivers/bluetooth/btqca.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/bluetooth/btqca.c b/drivers/bluetooth/btqca.c
> index 04ebe290bc78..10c496eaea2c 100644
> --- a/drivers/bluetooth/btqca.c
> +++ b/drivers/bluetooth/btqca.c
> @@ -415,7 +415,7 @@ static int qca_tlv_check_data(struct hci_dev *hdev,
>  
>  		idx = 0;
>  		data = tlv->data;
> -		while (idx < length - sizeof(struct tlv_type_nvm)) {
> +		while (idx + sizeof(struct tlv_type_nvm) <= length) {
>  			tlv_nvm = (struct tlv_type_nvm *)(data + idx);
>  
>  			tag_id = le16_to_cpu(tlv_nvm->tag_id);

With the commit message fixed you can add my:

Reviewed-by: Johan Hovold <johan@kernel.org>

Johan