[PATCH BlueZ] a2dp: Fix loading of remote SEP from cache

[PATCH BlueZ] a2dp: Fix loading of remote SEP from cache

[Mikhail Gavrilov]

Commit 912f5efb0dd9 ("a2dp: Fix handling of codec capability storage")
added an 'i >= 2' condition to the loop that parses the stored codec
capabilities. Since i starts at 0 the loop body never runs, i stays 0,
and the subsequent 'if (i != size)' check discards every endpoint found
in the cache:

  load_remote_sep() Unable to load LastUsed: rseid 6 not found

With no remote SEP loaded, avdtp_discover() can no longer take the
cached path and issues AVDTP Discover/Get All Capabilities on every
reconnect, and a2dp_setup_remote_path() returns NULL so the transport
ends up at /org/bluez/hciX/dev_YY/fd0 instead of .../sepN/fd0.

The condition was presumably meant to bound the writes into data[128],
which the original commit did not actually fix: caps is read with
'%512s' so size / 2 can be up to 256. Validate size up front instead.

Fixes: 912f5efb0dd9 ("a2dp: Fix handling of codec capability storage")
---

Reproduced on Fedora Rawhide with bluez-5.87-2.fc45 and a pair of
Denon PerL Pro TWS earbuds. Without cached remote SEPs the earbuds'
incoming AVDTP Set Configuration races with the discovery bluetoothd
now has to perform, the stream never leaves CONFIGURED, and any client
opening the sink stalls. Fixed with this patch.

 profiles/audio/a2dp.c | 13 ++++++++++---
 1 file changed, 10 insertions(+), 3 deletions(-)

diff --git a/profiles/audio/a2dp.c b/profiles/audio/a2dp.c
index c8adc3122..a4ba1dacf 100644
--- a/profiles/audio/a2dp.c
+++ b/profiles/audio/a2dp.c
@@ -2398,7 +2398,16 @@ static void load_remote_sep(struct a2dp_channel *chan, GKeyFile *key_file,
 			delay_reporting = false;
 		}
 
-		for (i = 0, size = strlen(caps); i < size && i >= 2; i += 2) {
+		size = strlen(caps);
+
+		g_free(value);
+
+		if (size % 2 || size / 2 > (int) sizeof(data)) {
+			warn("Unable to load Endpoint: seid %u", rseid);
+			continue;
+		}
+
+		for (i = 0; i < size; i += 2) {
 			uint8_t *tmp = data + i / 2;
 
 			if (sscanf(caps + i, "%02hhx", tmp) != 1) {
@@ -2407,8 +2416,6 @@ static void load_remote_sep(struct a2dp_channel *chan, GKeyFile *key_file,
 			}
 		}
 
-		g_free(value);
-
 		if (i != size)
 			continue;
 
-- 
2.55.0

RE: [BlueZ] a2dp: Fix loading of remote SEP from cache

[bluez.test.bot]

[-- Attachment #1: Type: text/plain, Size: 987 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=1125195

---Test result---

Test Summary:
CheckPatch                    PASS      0.34 seconds
GitLint                       PASS      0.22 seconds
BuildEll                      PASS      15.16 seconds
BluezMake                     PASS      369.21 seconds
MakeCheck                     PASS      3.33 seconds
MakeDistcheck                 PASS      119.51 seconds
CheckValgrind                 PASS      116.85 seconds
CheckSmatch                   PASS      204.82 seconds
bluezmakeextell               PASS      73.59 seconds
IncrementalBuild              PASS      372.53 seconds
ScanBuild                     PASS      644.20 seconds



https://github.com/bluez/bluez/pull/2290

---
Regards,
Linux Bluetooth

Re: [PATCH BlueZ] a2dp: Fix loading of remote SEP from cache

[Luiz Augusto von Dentz]

Hi Mikhail,

On Fri, Jul 10, 2026 at 3:28 AM Mikhail Gavrilov
<mikhail.v.gavrilov@gmail.com> wrote:
>
> Commit 912f5efb0dd9 ("a2dp: Fix handling of codec capability storage")
> added an 'i >= 2' condition to the loop that parses the stored codec
> capabilities. Since i starts at 0 the loop body never runs, i stays 0,
> and the subsequent 'if (i != size)' check discards every endpoint found
> in the cache:
>
>   load_remote_sep() Unable to load LastUsed: rseid 6 not found
>
> With no remote SEP loaded, avdtp_discover() can no longer take the
> cached path and issues AVDTP Discover/Get All Capabilities on every
> reconnect, and a2dp_setup_remote_path() returns NULL so the transport
> ends up at /org/bluez/hciX/dev_YY/fd0 instead of .../sepN/fd0.
>
> The condition was presumably meant to bound the writes into data[128],
> which the original commit did not actually fix: caps is read with
> '%512s' so size / 2 can be up to 256. Validate size up front instead.
>
> Fixes: 912f5efb0dd9 ("a2dp: Fix handling of codec capability storage")
> ---
>
> Reproduced on Fedora Rawhide with bluez-5.87-2.fc45 and a pair of
> Denon PerL Pro TWS earbuds. Without cached remote SEPs the earbuds'
> incoming AVDTP Set Configuration races with the discovery bluetoothd
> now has to perform, the stream never leaves CONFIGURED, and any client
> opening the sink stalls. Fixed with this patch.
>
>  profiles/audio/a2dp.c | 13 ++++++++++---
>  1 file changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/profiles/audio/a2dp.c b/profiles/audio/a2dp.c
> index c8adc3122..a4ba1dacf 100644
> --- a/profiles/audio/a2dp.c
> +++ b/profiles/audio/a2dp.c
> @@ -2398,7 +2398,16 @@ static void load_remote_sep(struct a2dp_channel *chan, GKeyFile *key_file,
>                         delay_reporting = false;
>                 }
>
> -               for (i = 0, size = strlen(caps); i < size && i >= 2; i += 2) {

Would it be simpler to just fix it with size >= 2?

> +               size = strlen(caps);
> +
> +               g_free(value);
> +
> +               if (size % 2 || size / 2 > (int) sizeof(data)) {
> +                       warn("Unable to load Endpoint: seid %u", rseid);
> +                       continue;
> +               }

Or is this just to warn the system that the endpoint cache is broken
or something?

> +               for (i = 0; i < size; i += 2) {
>                         uint8_t *tmp = data + i / 2;
>
>                         if (sscanf(caps + i, "%02hhx", tmp) != 1) {
> @@ -2407,8 +2416,6 @@ static void load_remote_sep(struct a2dp_channel *chan, GKeyFile *key_file,
>                         }
>                 }
>
> -               g_free(value);
> -
>                 if (i != size)
>                         continue;
>
> --
> 2.55.0
>
>


-- 
Luiz Augusto von Dentz

Re: [PATCH BlueZ] a2dp: Fix loading of remote SEP from cache

[patchwork-bot+bluetooth]

Hello:

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

On Fri, 10 Jul 2026 12:28:17 +0500 you wrote:
> Commit 912f5efb0dd9 ("a2dp: Fix handling of codec capability storage")
> added an 'i >= 2' condition to the loop that parses the stored codec
> capabilities. Since i starts at 0 the loop body never runs, i stays 0,
> and the subsequent 'if (i != size)' check discards every endpoint found
> in the cache:
> 
>   load_remote_sep() Unable to load LastUsed: rseid 6 not found
> 
> [...]

Here is the summary with links:
  - [BlueZ] a2dp: Fix loading of remote SEP from cache
    https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=b7d71e506785

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

Re: [PATCH BlueZ] a2dp: Fix loading of remote SEP from cache

[Mikhail Gavrilov]

On Fri, Jul 10, 2026 at 7:19 PM Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
>
> Would it be simpler to just fix it with size >= 2?
>

Not really. size is the length of the hex string in characters, not a
byte count, and the 'i >= 2' was evaluated on every iteration with i
starting at 0, so the body never ran at all. Replacing it with
'size >= 2' would move the test to the whole loop condition rather than
the entry, and the loop would still be walking a stale counter. The
entry guard and the body need to be separated, which is what splitting
the loop does.

>
> Or is this just to warn the system that the endpoint cache is broken
> or something?
>

Right, that part is not needed to fix the regression itself. It rejects
a malformed entry (odd length, or one that would overflow data[128]).
caps is read with '%512s', so size / 2 can be up to 256 bytes into a
128-byte buffer; the original commit meant to guard that but the check
never took effect. The revert of the regression is just moving g_free()
and dropping 'i >= 2'; the size check is added hardening for that
overflow. Happy to split it into a separate patch if you'd prefer to
keep the two concerns apart.

-- 
Thanks,
Mikhail