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

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

[Laxman Acharya Padhya]

The Read Local Supported Codecs parsers consume the variable-sized
standard codec array before parsing the vendor codec count.  Although the
initial reply-size check includes a vendor count byte in the fixed layout,
it does not guarantee that the byte remains after the standard codec array.

If a controller reply ends immediately after that array, calculating the
vendor codec array size reads vnd_codecs->num beyond the skb data.  Require
the vendor codec header to be present before using its count in both
command variants.

Fixes: 8961987f3f5f ("Bluetooth: Enumerate local supported codec and cache details")
Fixes: 9ae664028a9e ("Bluetooth: Add support for Read Local Supported Codecs V2")
Cc: stable@vger.kernel.org
Signed-off-by: Laxman Acharya Padhya <acharyalaxman8848@gmail.com>
---
 net/bluetooth/hci_codec.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/net/bluetooth/hci_codec.c b/net/bluetooth/hci_codec.c
index 5bc5003c387c..99394a3348c7 100644
--- a/net/bluetooth/hci_codec.c
+++ b/net/bluetooth/hci_codec.c
@@ -165,6 +165,8 @@ void hci_read_supported_codecs(struct hci_dev *hdev)
 		 + sizeof(std_codecs->num));
 
 	vnd_codecs = (void *)skb->data;
+	if (skb->len < sizeof(*vnd_codecs))
+		goto error;
 
 	/* validate vendor codecs length before accessing */
 	if (skb->len <
@@ -233,6 +235,8 @@ void hci_read_supported_codecs_v2(struct hci_dev *hdev)
 		 + sizeof(std_codecs->num));
 
 	vnd_codecs = (void *)skb->data;
+	if (skb->len < sizeof(*vnd_codecs))
+		goto error;
 
 	/* check for payload data length before accessing */
 	if (skb->len <
-- 
2.50.1

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

[Luiz Augusto von Dentz]

Hi Laxman,

On Sun, Aug 23, 2026 at 12:16 PM Laxman Acharya Padhya
<acharyalaxman8848@gmail.com> wrote:
>
> The Read Local Supported Codecs parsers consume the variable-sized
> standard codec array before parsing the vendor codec count.  Although the
> initial reply-size check includes a vendor count byte in the fixed layout,
> it does not guarantee that the byte remains after the standard codec array.
>
> If a controller reply ends immediately after that array, calculating the
> vendor codec array size reads vnd_codecs->num beyond the skb data.  Require
> the vendor codec header to be present before using its count in both
> command variants.
>
> Fixes: 8961987f3f5f ("Bluetooth: Enumerate local supported codec and cache details")
> Fixes: 9ae664028a9e ("Bluetooth: Add support for Read Local Supported Codecs V2")
> Cc: stable@vger.kernel.org
> Signed-off-by: Laxman Acharya Padhya <acharyalaxman8848@gmail.com>
> ---
>  net/bluetooth/hci_codec.c | 4 ++++
>  1 file changed, 4 insertions(+)
>
> diff --git a/net/bluetooth/hci_codec.c b/net/bluetooth/hci_codec.c
> index 5bc5003c387c..99394a3348c7 100644
> --- a/net/bluetooth/hci_codec.c
> +++ b/net/bluetooth/hci_codec.c
> @@ -165,6 +165,8 @@ void hci_read_supported_codecs(struct hci_dev *hdev)
>                  + sizeof(std_codecs->num));
>
>         vnd_codecs = (void *)skb->data;
> +       if (skb->len < sizeof(*vnd_codecs))
> +               goto error;

If Im not mistake the following code already validates skb->len:

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

sizeof(vnd_codecs->num) is equivalent to what you are proposing with
sizeof(*vnd_codecs), so this doesn't fix anything it just duplicates
the testing for the minimal size.

>         /* validate vendor codecs length before accessing */
>         if (skb->len <
> @@ -233,6 +235,8 @@ void hci_read_supported_codecs_v2(struct hci_dev *hdev)
>                  + sizeof(std_codecs->num));
>
>         vnd_codecs = (void *)skb->data;
> +       if (skb->len < sizeof(*vnd_codecs))
> +               goto error;
>
>         /* check for payload data length before accessing */
>         if (skb->len <
> --
> 2.50.1



-- 
Luiz Augusto von Dentz

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

[Laxman Acharya Padhya]

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.

Thanks,
Laxman

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

[Luiz Augusto von Dentz]

Hi Laxman,

On Mon, Aug 24, 2026 at 10:40 AM Laxman Acharya Padhya
<acharyalaxman8848@gmail.com> 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

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

[Laxman Acharya Padhya]

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))

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.

Thanks,
Laxman

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

[Luiz Augusto von Dentz]

Hi Laxman,

On Mon, Aug 24, 2026 at 11:20 AM Laxman Acharya Padhya
<acharyalaxman8848@gmail.com> 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

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

[Laxman Acharya Padhya]

The Read Local Supported Codecs parsers consume the variable-sized
standard codec array before parsing the vendor codec count.  Although the
initial reply-size check includes a vendor count byte in the fixed layout,
it does not guarantee that the byte remains after the standard codec array.

If a controller reply ends immediately after that array, calculating the
vendor codec array size reads vnd_codecs->num beyond the skb data.  Use
skb_pull_data() to validate and consume each codec header before using its
count in both command variants.

Fixes: 8961987f3f5f ("Bluetooth: Enumerate local supported codec and cache details")
Fixes: 9ae664028a9e ("Bluetooth: Add support for Read Local Supported Codecs V2")
Cc: stable@vger.kernel.org
Suggested-by: Luiz Augusto von Dentz <luiz.dentz@gmail.com>
Signed-off-by: Laxman Acharya Padhya <acharyalaxman8848@gmail.com>
---
Changes in v2:
- Use skb_pull_data() to validate and consume the standard and vendor
  codec headers before accessing their counts, as suggested by Luiz.

 net/bluetooth/hci_codec.c | 36 ++++++++++++++++++------------------
 1 file changed, 18 insertions(+), 18 deletions(-)

diff --git a/net/bluetooth/hci_codec.c b/net/bluetooth/hci_codec.c
index 5bc5003c387c..7a7e813dcdda 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 */
@@ -214,11 +214,12 @@ void hci_read_supported_codecs_v2(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;
 
 	/* check for payload data 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;
 
 	memset(&caps, 0, sizeof(caps));
@@ -229,15 +230,14 @@ void hci_read_supported_codecs_v2(struct hci_dev *hdev)
 					    &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;
 
 	/* check for payload data 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;
 
 	for (i = 0; i < vnd_codecs->num; i++) {
-- 
2.51.2

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

[patchwork-bot+bluetooth]

Hello:

This patch was applied to bluetooth/bluetooth-next.git (master)
by Luiz Augusto von Dentz <luiz.von.dentz@intel.com>:

On Mon, 24 Aug 2026 21:42:36 +0545 you wrote:
> The Read Local Supported Codecs parsers consume the variable-sized
> standard codec array before parsing the vendor codec count.  Although the
> initial reply-size check includes a vendor count byte in the fixed layout,
> it does not guarantee that the byte remains after the standard codec array.
> 
> If a controller reply ends immediately after that array, calculating the
> vendor codec array size reads vnd_codecs->num beyond the skb data.  Use
> skb_pull_data() to validate and consume each codec header before using its
> count in both command variants.
> 
> [...]

Here is the summary with links:
  - [v2] Bluetooth: hci_codec: validate vendor codec count length
    https://git.kernel.org/bluetooth/bluetooth-next/c/803d146cd56b

You are awesome, thank you!
-- 
Deet-doot-dot, I am a bot.
https://korg.docs.kernel.org/patchwork/pwbot.html