[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: Bluetooth: hci_codec: validate vendor codec count length

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 2389 bytes --]

This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=1150464

---Test result---

Test Summary:
CheckPatch                    PASS      0.60 seconds
VerifyFixes                   PASS      0.10 seconds
VerifySignedoff               PASS      0.10 seconds
GitLint                       PASS      0.25 seconds
SubjectPrefix                 PASS      0.09 seconds
BuildKernel                   PASS      25.30 seconds
CheckAllWarning               PASS      27.63 seconds
CheckSparse                   PASS      26.36 seconds
BuildKernel32                 PASS      24.39 seconds
CheckKernelLLVM               SKIP      0.00 seconds
TestRunnerSetup               PASS      461.06 seconds
TestRunner_l2cap-tester       PASS      68.63 seconds
TestRunner_iso-tester         PASS      80.81 seconds
TestRunner_bnep-tester        PASS      18.78 seconds
TestRunner_mgmt-tester        FAIL      226.98 seconds
TestRunner_rfcomm-tester      PASS      25.28 seconds
TestRunner_sco-tester         PASS      33.01 seconds
TestRunner_ioctl-tester       PASS      26.44 seconds
TestRunner_mesh-tester        FAIL      25.89 seconds
TestRunner_smp-tester         PASS      23.25 seconds
TestRunner_userchan-tester    PASS      19.90 seconds
TestRunner_6lowpan-tester     PASS      22.91 seconds
IncrementalBuild              PASS      23.20 seconds

Details
##############################
Test: CheckKernelLLVM - SKIP
Desc: Build kernel with LLVM + context analysis
Output:
Clang not found
##############################
Test: TestRunner_mgmt-tester - FAIL
Desc: Run mgmt-tester with test-runner
Output:
Total: 501, Passed: 496 (99.0%), Failed: 1, Not Run: 4

Failed Test Cases
Read Exp Feature - Success                           Failed       0.256 seconds
##############################
Test: TestRunner_mesh-tester - FAIL
Desc: Run mesh-tester with test-runner
Output:
Total: 10, Passed: 8 (80.0%), Failed: 2, Not Run: 0

Failed Test Cases
Mesh - Send cancel - 1                               Timed out    2.567 seconds
Mesh - Send cancel - 2                               Timed out    1.987 seconds


https://github.com/bluez/bluetooth-next/pull/638

---
Regards,
Linux Bluetooth

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: [v2] Bluetooth: hci_codec: validate vendor codec count length

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 2390 bytes --]

This is automated email and please do not reply to this email!

Dear submitter,

Thank you for submitting the patches to the linux bluetooth mailing list.
This is a CI test results with your patch series:
PW Link:https://patchwork.kernel.org/project/bluetooth/list/?series=1151041

---Test result---

Test Summary:
CheckPatch                    PASS      0.82 seconds
VerifyFixes                   PASS      0.07 seconds
VerifySignedoff               PASS      0.07 seconds
GitLint                       PASS      0.21 seconds
SubjectPrefix                 PASS      0.06 seconds
BuildKernel                   PASS      27.91 seconds
CheckAllWarning               PASS      30.87 seconds
CheckSparse                   PASS      29.80 seconds
BuildKernel32                 PASS      26.96 seconds
CheckKernelLLVM               SKIP      0.00 seconds
TestRunnerSetup               PASS      511.79 seconds
TestRunner_l2cap-tester       PASS      67.63 seconds
TestRunner_iso-tester         PASS      107.24 seconds
TestRunner_bnep-tester        PASS      19.71 seconds
TestRunner_mgmt-tester        FAIL      225.37 seconds
TestRunner_rfcomm-tester      PASS      25.88 seconds
TestRunner_sco-tester         PASS      32.26 seconds
TestRunner_ioctl-tester       PASS      27.28 seconds
TestRunner_mesh-tester        FAIL      26.95 seconds
TestRunner_smp-tester         PASS      24.28 seconds
TestRunner_userchan-tester    PASS      20.92 seconds
TestRunner_6lowpan-tester     PASS      24.71 seconds
IncrementalBuild              PASS      26.36 seconds

Details
##############################
Test: CheckKernelLLVM - SKIP
Desc: Build kernel with LLVM + context analysis
Output:
Clang not found
##############################
Test: TestRunner_mgmt-tester - FAIL
Desc: Run mgmt-tester with test-runner
Output:
Total: 501, Passed: 496 (99.0%), Failed: 1, Not Run: 4

Failed Test Cases
Read Exp Feature - Success                           Failed       0.260 seconds
##############################
Test: TestRunner_mesh-tester - FAIL
Desc: Run mesh-tester with test-runner
Output:
Total: 10, Passed: 8 (80.0%), Failed: 2, Not Run: 0

Failed Test Cases
Mesh - Send cancel - 1                               Timed out    2.264 seconds
Mesh - Send cancel - 2                               Timed out    1.992 seconds


https://github.com/bluez/bluetooth-next/pull/646

---
Regards,
Linux Bluetooth