[PATCH] client/player: Refcount registered endpoints

[PATCH] client/player: Refcount registered endpoints

[Arkadiusz Bokowy]

In case when more than one adapter is available on the host, the
endpoint is registered on all of these adapters. When unregistering
we need to keep track of registration count, otherwise we might free
the endpoint structure prematurely (on the first unregister reply)
which will cause SIGSEGV.
---
 client/player.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/client/player.c b/client/player.c
index b6b02a000..cedb07d65 100644
--- a/client/player.c
+++ b/client/player.c
@@ -127,6 +127,7 @@ struct endpoint {
 	struct codec_preset *codec_preset;
 	bool broadcast;
 	struct iovec *bcode;
+	unsigned int refcount;
 };
 
 static DBusConnection *dbus_conn;
@@ -3323,6 +3324,7 @@ static void register_endpoint_reply(DBusMessage *message, void *user_data)
 	}
 
 	bt_shell_printf("Endpoint %s registered\n", ep->path);
+	ep->refcount++;
 
 	return bt_shell_noninteractive_quit(EXIT_SUCCESS);
 }
@@ -3737,9 +3739,13 @@ static void unregister_endpoint_reply(DBusMessage *message, void *user_data)
 
 	bt_shell_printf("Endpoint %s unregistered\n", ep->path);
 
-	local_endpoints = g_list_remove(local_endpoints, ep);
-	g_dbus_unregister_interface(dbus_conn, ep->path,
-					BLUEZ_MEDIA_ENDPOINT_INTERFACE);
+	ep->refcount--;
+
+	if (ep->refcount == 0) {
+		local_endpoints = g_list_remove(local_endpoints, ep);
+		g_dbus_unregister_interface(dbus_conn, ep->path,
+					    BLUEZ_MEDIA_ENDPOINT_INTERFACE);
+	}
 
 	return bt_shell_noninteractive_quit(EXIT_SUCCESS);
 }
-- 
2.51.0

RE: client/player: Refcount registered endpoints

[bluez.test.bot]

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

---Test result---

Test Summary:
CheckPatch                    PENDING   0.50 seconds
GitLint                       PENDING   0.41 seconds
BuildEll                      PASS      20.18 seconds
BluezMake                     PASS      644.55 seconds
MakeCheck                     PASS      21.77 seconds
MakeDistcheck                 PASS      242.84 seconds
CheckValgrind                 PASS      306.70 seconds
CheckSmatch                   PASS      352.61 seconds
bluezmakeextell               PASS      183.35 seconds
IncrementalBuild              PENDING   0.42 seconds
ScanBuild                     PASS      1033.70 seconds

Details
##############################
Test: CheckPatch - PENDING
Desc: Run checkpatch.pl script
Output:

##############################
Test: GitLint - PENDING
Desc: Run gitlint
Output:

##############################
Test: IncrementalBuild - PENDING
Desc: Incremental build with the patches in the series
Output:



---
Regards,
Linux Bluetooth

Re: [PATCH] client/player: Refcount registered endpoints

[Luiz Augusto von Dentz]

Hi Arkadiusz,

On Mon, Dec 15, 2025 at 9:58 AM Arkadiusz Bokowy
<arkadiusz.bokowy@gmail.com> wrote:
>
> In case when more than one adapter is available on the host, the
> endpoint is registered on all of these adapters. When unregistering

org.bluez.Media is per adapter, so the likes of RegisterEndpoint
registers a new object per adapter.

> we need to keep track of registration count, otherwise we might free
> the endpoint structure prematurely (on the first unregister reply)
> which will cause SIGSEGV.

Do you have a backtrace or something? We don't reference struct
endpoint across different adapters afaik so this might be a different
bug or something.

> ---
>  client/player.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)
>
> diff --git a/client/player.c b/client/player.c
> index b6b02a000..cedb07d65 100644
> --- a/client/player.c
> +++ b/client/player.c
> @@ -127,6 +127,7 @@ struct endpoint {
>         struct codec_preset *codec_preset;
>         bool broadcast;
>         struct iovec *bcode;
> +       unsigned int refcount;
>  };
>
>  static DBusConnection *dbus_conn;
> @@ -3323,6 +3324,7 @@ static void register_endpoint_reply(DBusMessage *message, void *user_data)
>         }
>
>         bt_shell_printf("Endpoint %s registered\n", ep->path);
> +       ep->refcount++;
>
>         return bt_shell_noninteractive_quit(EXIT_SUCCESS);
>  }
> @@ -3737,9 +3739,13 @@ static void unregister_endpoint_reply(DBusMessage *message, void *user_data)
>
>         bt_shell_printf("Endpoint %s unregistered\n", ep->path);
>
> -       local_endpoints = g_list_remove(local_endpoints, ep);
> -       g_dbus_unregister_interface(dbus_conn, ep->path,
> -                                       BLUEZ_MEDIA_ENDPOINT_INTERFACE);
> +       ep->refcount--;
> +
> +       if (ep->refcount == 0) {
> +               local_endpoints = g_list_remove(local_endpoints, ep);
> +               g_dbus_unregister_interface(dbus_conn, ep->path,
> +                                           BLUEZ_MEDIA_ENDPOINT_INTERFACE);
> +       }
>
>         return bt_shell_noninteractive_quit(EXIT_SUCCESS);
>  }
> --
> 2.51.0
>
>


-- 
Luiz Augusto von Dentz

Re: [PATCH] client/player: Refcount registered endpoints

[Arkadiusz Bokowy]

> Do you have a backtrace or something? We don't reference struct
> endpoint across different adapters afaik so this might be a different
> bug or something.

> [bluetoothctl]> endpoint.register 0000110a-0000-1000-8000-00805f9b34fb 0x00 '0xFF 0xFF 0x02 0x40'
> [/local/endpoint/ep0] Enter Metadata (value/no): no
> [/local/endpoint/ep0] Auto Accept (yes/no): no
> [/local/endpoint/ep0] Max Transports (auto/value): auto
> [/local/endpoint/ep0] Locations: 0
> [/local/endpoint/ep0] Supported Context (value): 0
> [/local/endpoint/ep0] Context (value): 0
> [/local/endpoint/ep0] CIG (auto/value): auto
> [/local/endpoint/ep0] CIS (auto/value): auto
> Capabilities:
>   ff ff 02 40                                      ...@
> Capabilities:
>   ff ff 02 40                                      ...@
> Endpoint /local/endpoint/ep0 registered
> Endpoint /local/endpoint/ep0 registered
> [bluetoothctl]> endpoint.unregister /local/endpoint/ep0
> Endpoint /local/endpoint/ep0 unregistered
> [bluetoothctl]>
> Program received signal SIGSEGV, Segmentation fault.
> Download failed: Invalid argument.  Continuing without source file ./string/../sysdeps/x86_64/multiarch/strlen-avx2.S.
> __strlen_avx2 () at ../sysdeps/x86_64/multiarch/strlen-avx2.S:76
> warning: 76     ../sysdeps/x86_64/multiarch/strlen-avx2.S: No such file or directory
> (gdb) bt
> #0  __strlen_avx2 () at ../sysdeps/x86_64/multiarch/strlen-avx2.S:76
> #1  0x00007ffff7a6f980 in __printf_buffer (buf=buf@entry=0x7fffffffc7a0, format=format@entry=0x5555555afbd3 "Endpoint %s unregistered\n", ap=ap@entry=0x7fffffffc8b0, mode_f
>     at ./stdio-common/vfprintf-process-arg.c:443
> #2  0x00007ffff7a70788 in __vfprintf_internal (s=0x7ffff7c355c0 <_IO_2_1_stdout_>, format=format@entry=0x5555555afbd3 "Endpoint %s unregistered\n", ap=ap@entry=0x7fffffffc8
>     at ./stdio-common/vfprintf-internal.c:1543
> #3  0x00007ffff7b45489 in ___vfprintf_chk (fp=<optimized out>, flag=flag@entry=2, format=format@entry=0x5555555afbd3 "Endpoint %s unregistered\n", ap=ap@entry=0x7fffffffc8b
> #4  0x00005555555a90bf in vprintf (__fmt=0x5555555afbd3 "Endpoint %s unregistered\n", __ap=0x7fffffffc8b0) at /usr/include/x86_64-linux-gnu/bits/stdio2.h:156
> #5  bt_shell_printf (fmt=fmt@entry=0x5555555afbd3 "Endpoint %s unregistered\n") at ../src/shared/shell.c:741
> #6  0x0000555555587739 in unregister_endpoint_reply (message=0x5555562c2990, user_data=0x5555562b9c00) at ../client/player.c:3738
> #7  unregister_endpoint_reply (message=0x5555562c2990, user_data=0x5555562b9c00) at ../client/player.c:3724
> #8  0x00005555555a1516 in method_call_reply (call=<optimized out>, user_data=0x5555562c2270) at ../gdbus/client.c:1075
> #9  0x00007ffff7e037c2 in ?? () from /lib/x86_64-linux-gnu/libdbus-1.so.3
> #10 0x00007ffff7e07dfe in dbus_connection_dispatch () from /lib/x86_64-linux-gnu/libdbus-1.so.3
> #11 0x000055555559c828 in message_dispatch (data=0x5555562a8890) at ../gdbus/mainloop.c:59
> #12 0x00007ffff7e9fbfb in ?? () from /lib/x86_64-linux-gnu/libglib-2.0.so.0
> #13 0x00007ffff7ea1237 in ?? () from /lib/x86_64-linux-gnu/libglib-2.0.so.0
> #14 0x00007ffff7ea15d7 in g_main_loop_run () from /lib/x86_64-linux-gnu/libglib-2.0.so.0
> #15 0x00005555555ab7f9 in mainloop_run () at ../src/shared/mainloop-glib.c:65
> #16 0x00005555555abbfa in mainloop_run_with_signal (func=func@entry=0x5555555a8880 <signal_callback>, user_data=user_data@entry=0x0) at ../src/shared/mainloop-notify.c:196
> #17 0x00005555555aa655 in bt_shell_run () at ../src/shared/shell.c:1476
> #18 0x000055555556f418 in main (argc=<optimized out>, argv=0x7fffffffcef8) at ../client/main.c:3960

The bug is caused by freeing the endpoint on the first reply from
deregistration. When the second one (or another if you have 3, 4, 5..
etc. adapters) arrieves you have access after free. Simple refcount
should fix that, but of course there might be other approaches.

Re: [PATCH] client/player: Refcount registered endpoints

[Arkadiusz Bokowy]

> org.bluez.Media is per adapter, so the likes of RegisterEndpoint
> registers a new object per adapter.

Or maybe the /local/endpoint/ep0 should be per-adapter in this player
code? I have not checked whether it works with many adapters, though.
My use case is that I'm not able to deregister media endpoint in case
I have more than one adapter attached to my host.

Re: [PATCH] client/player: Refcount registered endpoints

[Luiz Augusto von Dentz]

Hi Arkadiusz,

On Mon, Dec 15, 2025 at 10:33 AM Arkadiusz Bokowy
<arkadiusz.bokowy@gmail.com> wrote:
>
> > Do you have a backtrace or something? We don't reference struct
> > endpoint across different adapters afaik so this might be a different
> > bug or something.
>
> > [bluetoothctl]> endpoint.register 0000110a-0000-1000-8000-00805f9b34fb 0x00 '0xFF 0xFF 0x02 0x40'
> > [/local/endpoint/ep0] Enter Metadata (value/no): no
> > [/local/endpoint/ep0] Auto Accept (yes/no): no
> > [/local/endpoint/ep0] Max Transports (auto/value): auto
> > [/local/endpoint/ep0] Locations: 0
> > [/local/endpoint/ep0] Supported Context (value): 0
> > [/local/endpoint/ep0] Context (value): 0
> > [/local/endpoint/ep0] CIG (auto/value): auto
> > [/local/endpoint/ep0] CIS (auto/value): auto
> > Capabilities:
> >   ff ff 02 40                                      ...@
> > Capabilities:
> >   ff ff 02 40                                      ...@
> > Endpoint /local/endpoint/ep0 registered
> > Endpoint /local/endpoint/ep0 registered
> > [bluetoothctl]> endpoint.unregister /local/endpoint/ep0
> > Endpoint /local/endpoint/ep0 unregistered
> > [bluetoothctl]>
> > Program received signal SIGSEGV, Segmentation fault.
> > Download failed: Invalid argument.  Continuing without source file ./string/../sysdeps/x86_64/multiarch/strlen-avx2.S.
> > __strlen_avx2 () at ../sysdeps/x86_64/multiarch/strlen-avx2.S:76
> > warning: 76     ../sysdeps/x86_64/multiarch/strlen-avx2.S: No such file or directory
> > (gdb) bt
> > #0  __strlen_avx2 () at ../sysdeps/x86_64/multiarch/strlen-avx2.S:76
> > #1  0x00007ffff7a6f980 in __printf_buffer (buf=buf@entry=0x7fffffffc7a0, format=format@entry=0x5555555afbd3 "Endpoint %s unregistered\n", ap=ap@entry=0x7fffffffc8b0, mode_f
> >     at ./stdio-common/vfprintf-process-arg.c:443
> > #2  0x00007ffff7a70788 in __vfprintf_internal (s=0x7ffff7c355c0 <_IO_2_1_stdout_>, format=format@entry=0x5555555afbd3 "Endpoint %s unregistered\n", ap=ap@entry=0x7fffffffc8
> >     at ./stdio-common/vfprintf-internal.c:1543
> > #3  0x00007ffff7b45489 in ___vfprintf_chk (fp=<optimized out>, flag=flag@entry=2, format=format@entry=0x5555555afbd3 "Endpoint %s unregistered\n", ap=ap@entry=0x7fffffffc8b
> > #4  0x00005555555a90bf in vprintf (__fmt=0x5555555afbd3 "Endpoint %s unregistered\n", __ap=0x7fffffffc8b0) at /usr/include/x86_64-linux-gnu/bits/stdio2.h:156
> > #5  bt_shell_printf (fmt=fmt@entry=0x5555555afbd3 "Endpoint %s unregistered\n") at ../src/shared/shell.c:741
> > #6  0x0000555555587739 in unregister_endpoint_reply (message=0x5555562c2990, user_data=0x5555562b9c00) at ../client/player.c:3738
> > #7  unregister_endpoint_reply (message=0x5555562c2990, user_data=0x5555562b9c00) at ../client/player.c:3724
> > #8  0x00005555555a1516 in method_call_reply (call=<optimized out>, user_data=0x5555562c2270) at ../gdbus/client.c:1075
> > #9  0x00007ffff7e037c2 in ?? () from /lib/x86_64-linux-gnu/libdbus-1.so.3
> > #10 0x00007ffff7e07dfe in dbus_connection_dispatch () from /lib/x86_64-linux-gnu/libdbus-1.so.3
> > #11 0x000055555559c828 in message_dispatch (data=0x5555562a8890) at ../gdbus/mainloop.c:59
> > #12 0x00007ffff7e9fbfb in ?? () from /lib/x86_64-linux-gnu/libglib-2.0.so.0
> > #13 0x00007ffff7ea1237 in ?? () from /lib/x86_64-linux-gnu/libglib-2.0.so.0
> > #14 0x00007ffff7ea15d7 in g_main_loop_run () from /lib/x86_64-linux-gnu/libglib-2.0.so.0
> > #15 0x00005555555ab7f9 in mainloop_run () at ../src/shared/mainloop-glib.c:65
> > #16 0x00005555555abbfa in mainloop_run_with_signal (func=func@entry=0x5555555a8880 <signal_callback>, user_data=user_data@entry=0x0) at ../src/shared/mainloop-notify.c:196
> > #17 0x00005555555aa655 in bt_shell_run () at ../src/shared/shell.c:1476
> > #18 0x000055555556f418 in main (argc=<optimized out>, argv=0x7fffffffcef8) at ../client/main.c:3960

Oh, this is bluetoothctl bug, sorry I was looking into bluetoothd not
bluetoothctl, endpoint_register does indeed make a reference to the
same object.

> The bug is caused by freeing the endpoint on the first reply from
> deregistration. When the second one (or another if you have 3, 4, 5..
> etc. adapters) arrieves you have access after free. Simple refcount
> should fix that, but of course there might be other approaches.



-- 
Luiz Augusto von Dentz

Re: [PATCH] client/player: Refcount registered endpoints

[patchwork-bot+bluetooth]

Hello:

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

On Mon, 15 Dec 2025 15:02:03 +0100 you wrote:
> In case when more than one adapter is available on the host, the
> endpoint is registered on all of these adapters. When unregistering
> we need to keep track of registration count, otherwise we might free
> the endpoint structure prematurely (on the first unregister reply)
> which will cause SIGSEGV.
> ---
>  client/player.c | 12 +++++++++---
>  1 file changed, 9 insertions(+), 3 deletions(-)

Here is the summary with links:
  - client/player: Refcount registered endpoints
    https://git.kernel.org/pub/scm/bluetooth/bluez.git/?id=ba4978255c3c

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