Re: [PATCH] Bluetooth: btintel: validate version TLV value lengths

Hi,

On Wed, Aug 12, 2026 at 10:59:34PM +0545, Laxman Acharya Padhya wrote:
> btintel_parse_version_tlv() verifies that a complete TLV is present in
> the response, but it does not ensure that the value is long enough for
> the specific TLV type.

The analysis and the fix look correct to me. I also reproduced the issue
in userspace by lifting the parse loop as-is and running it under ASan,
with the skb data backed by a heap allocation sized exactly to the
received event.

A few malformed inputs trigger out-of-bounds reads with the current
code:

CNVI_TOP len=0 -> 4-byte read past the event
DEV_REV_ID len=1 -> 2-byte read past the event
BUILD_NUM len=1 -> 4-byte read past the event
OTP_BDADDR len=2 -> 6-byte read past the event
FW_ID without a NUL terminator -> scan past the TLV value

With the patch applied, the fixed-size cases return -EINVAL and FW_ID is
bounded by tlv->len. Well-formed input behaves the same as before.

I think the Fixes tag should be split, though, since these two issues
were introduced by different commits.

The FW_ID "%s" -> "%.*s" change fixes code added by

164c62f958f8 ("Bluetooth: btintel: Add firmware ID to firmware name")

which landed in v6.11. INTEL_TLV_FW_ID is not present in v5.15, v6.1, or
v6.6, so using a single

Fixes: ca5425e15881

would cause stable to try applying that hunk to trees where the code
does not exist.

The missing per-type length checks go back further. They were already
missing in

57375beef71a ("Bluetooth: btintel: Add infrastructure to read controller information")

which landed in v5.10. ca5425e15881 later added the outer TLV framing
checks, but not the per-type minimum length validation.

So I would split this into two patches:

1/2 add per-type minimum length validation
Fixes: 57375beef71a ("Bluetooth: btintel: Add infrastructure to read controller information")

2/2 bound FW_ID conversion to tlv->len
Fixes: 164c62f958f8 ("Bluetooth: btintel: Add firmware ID to firmware name")

One other thing about this hunk:

> -	btintel_parse_version_tlv(hdev, version, skb);
> +	err = btintel_parse_version_tlv(hdev, version, skb);
>
>  	kfree_skb(skb);
> -	return 0;
> +	return err;

This changes behaviour beyond fixing the out-of-bounds access. A
malformed TLV is currently ignored, while after this change it makes
setup fail.

That may well be the right behaviour, but I would consider putting it in
a separate patch. That way, if some existing controller turns out to
send malformed data, the setup failure can be reverted independently
without reverting the validation itself.

With the Fixes tags split, feel free to add:

Reviewed-by: Ali Ahmet Memis <[email protected]>

Thanks,
Ali