Re: [PATCH v1] Bluetooth: btintel: validate DDC record lengths

[Jeremy Clifton <deaner92@yahoo.com>]

Sai,

If you are busy, I am willing to
send a follow-up patch myself with cmd_plen widened to unsigned int,
referencing your original patch.

Thoughts? Is that proper etiquette?

Jeremy Dean

On 9/22/26 8:43 AM, Aluvala Sai Teja wrote:
> Sai,
> 
> A couple of things could be worth a second look here.
> 
> cmd_plen is still declared as u8:
> 
>     u8 cmd_plen = fw_ptr[0] + 1U;
> 
> The addition itself happens in a wider type due to integer promotion, but the result is truncated back down the moment it's stored in cmd_plen.
> 
> So if fw_ptr[0] is 0xFF, cmd_plen still wraps to 0, the same as before this patch. The commit message says this parses the length "in unsigned storage so 0xFF does not wrap". It will wrap as soon as it is placed back into the u8 cmd_plen variable.
> [Sai] : Agreed .will change cmd_plen declaration to unsigned int in v2 patch
> 
> It happens to get caught afterward by the cmd_plen < 3 check, since a wrapped value of 0 is less than 3. Is that the intended fix?
> [Sai] : Yes. DDC format is Length (1 byte), DDC ID (2 bytes), and DDC value . length and ddc id of 2 bytes are minimum expected bytes.
> The commit message seems to mismatch what is diffed.
> 
> Also, the cmd_plen > U8_MAX check can never be true. Since cmd_plen is a u8, its value is bounded by U8_MAX by definition.
> There's no input that will make this branch fire.
> [Sai] : Agreed. This check will become valid once cmd_plen is declared as unsigned int
> 
> Regards,
> Jeremy Dean
> ~