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

Hi Laxman,

On Mon, Aug 24, 2026 at 11:20 AM Laxman Acharya Padhya
<[email protected]> wrote:
>
> Hi Luiz,
>
> Thanks for looking at this.  Sorry that my earlier wording was unclear.
> I agree that the uses of p inside sizeof() and __must_be_array() are
> unevaluated, and that sizeof(vnd_codecs->num) is a compile-time value of
> one.
>
> The runtime access comes from the third macro argument, count.  Here that
> argument is the expression vnd_codecs->num.  Since it is not a constant
> expression, __builtin_choose_expr() selects the size_mul() branch, which
> effectively evaluates:
>
>         size_mul(vnd_codecs->num, sizeof(*vnd_codecs->codec))

Opps, yeah that indeed access the ->num.

> Evaluating the first argument reads vnd_codecs->num before the result is
> compared with skb->len.  Therefore, if no byte remains after the standard
> codec array, the read is already out of bounds.  The issue is this count
> load, rather than any evaluation of the sizeof() operands.
>
> I also verified this with a minimal reproducer using the same macro
> expansion: the compiler emits a byte load from vnd_codecs, and ASan
> reports a one-byte out-of-bounds read when the pointer is at the end of
> the buffer.
>
> Changing the trailing sizeof(vnd_codecs->num) to sizeof(*vnd_codecs)
> would give the same size, but it would not prevent the earlier count
> load.  The added check ensures that the count byte is present before
> flex_array_size() uses it.

That said I rather use skb_pull_data then:

diff --git a/net/bluetooth/hci_codec.c b/net/bluetooth/hci_codec.c
index 5bc5003c387c..fdc9652dad29 100644
--- a/net/bluetooth/hci_codec.c
+++ b/net/bluetooth/hci_codec.c
@@ -145,11 +145,12 @@ void hci_read_supported_codecs(struct hci_dev *hdev)

        skb_pull(skb, sizeof(rp->status));

-       std_codecs = (void *)skb->data;
+       std_codecs = skb_pull_data(skb, sizeof(*std_codecs));
+       if (!std_codecs)
+               goto error;

        /* validate codecs length before accessing */
-       if (skb->len < flex_array_size(std_codecs, codec, std_codecs->num)
-           + sizeof(std_codecs->num))
+       if (skb->len < flex_array_size(std_codecs, codec, std_codecs->num))
                goto error;

        /* enumerate codec capabilities of standard codecs */
@@ -161,15 +162,14 @@ void hci_read_supported_codecs(struct hci_dev *hdev)
                                            LOCAL_CODEC_ACL_MASK |
LOCAL_CODEC_SCO_MASK, &caps);
        }

-       skb_pull(skb, flex_array_size(std_codecs, codec, std_codecs->num)
-                + sizeof(std_codecs->num));
+       skb_pull(skb, flex_array_size(std_codecs, codec, std_codecs->num));

-       vnd_codecs = (void *)skb->data;
+       vnd_codecs = skb_pull_data(skb, sizeof(*vnd_codecs));
+       if (!vnd_codecs)
+               goto error;

        /* validate vendor codecs length before accessing */
-       if (skb->len <
-           flex_array_size(vnd_codecs, codec, vnd_codecs->num)
-           + sizeof(vnd_codecs->num))
+       if (skb->len < flex_array_size(vnd_codecs, codec, vnd_codecs->num))
                goto error;

        /* enumerate vendor codec capabilities */


-- 
Luiz Augusto von Dentz