Re: [PATCH] Bluetooth: hci_codec: validate vendor codec count length

Hi Laxman,

On Mon, Aug 24, 2026 at 10:40 AM Laxman Acharya Padhya
<[email protected]> wrote:
>
> Hi Luiz,
>
> You are right that sizeof(*vnd_codecs) and
> sizeof(vnd_codecs->num) are both one byte here.  The issue is the order
> in which the existing check performs the access: vnd_codecs->num must
> be evaluated as the count argument to flex_array_size() before the
> result can be compared with skb->len.
>
> For example, consider a V1 reply containing exactly these three bytes:
>
>         status = 0, std_codecs->num = 1, std_codecs->codec[0]
>
> It passes the initial sizeof(*rp) check.  After pulling the status byte,
> the standard codec length check also passes, and pulling that array
> leaves skb->len equal to zero.  The existing vendor length check then
> evaluates vnd_codecs->num with vnd_codecs pointing at the end of the
> skb data.
>
> The added check uses sizeof(), whose operand is not evaluated, to make
> sure the count byte is present before the following expression reads
> vnd_codecs->num.  The V2 parser has the same ordering issue.

So you are saying flex_array_size being performed before size_of would
matter? Afaike flex_array_size doesn't acuatually access the
vnd_codecs pointer, it just calculate the array size based on the it
size:

#define flex_array_size(p, member, count) \
__builtin_choose_expr(__is_constexpr(count), \
(count) * sizeof(*(p)->member) + __must_be_array((p)->member), \
size_mul(count, sizeof(*(p)->member) + __must_be_array((p)->member)))

Same thing as to sizeof(vnd_codecs->num) that would be evaluated to 1
at build time, so it doesn't generate an access to vnd_codecs at
runtime, maybe it would have been better to change it to
sizeof(*vnd_codecs) if that causes less confusion.

-- 
Luiz Augusto von Dentz