Hello!
Bluez bluetoothd daemon compiled from git master branch crashes when I
try to call DBus method for switching A2DP codec. Below is stacktrace
from gdb. It looks like NULL pointer dereference. It is reproducible.
Program received signal SIGSEGV, Segmentation fault.
0x000055e1b3659c1a in avdtp_find_remote_sep (session=0x55e1b408bf80, lsep=0x0) at profiles/audio/avdtp.c:1221
1221 if (lsep->info.inuse)
(gdb) bt
#0 0x000055e1b3659c1a in avdtp_find_remote_sep (session=0x55e1b408bf80, lsep=0x0) at profiles/audio/avdtp.c:1221
#1 0x000055e1b36568fc in find_remote_sep (sep=<optimized out>, chan=<optimized out>, chan=<optimized out>) at profiles/audio/a2dp.c:1169
#2 0x000055e1b3656955 in a2dp_reconfigure (data=0x55e1b40a1e10) at profiles/audio/a2dp.c:1188
#3 0x00007f4e07e90863 in ?? () from /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0
#4 0x00007f4e07e8fdd8 in g_main_context_dispatch () from /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0
#5 0x00007f4e07e901c8 in ?? () from /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0
#6 0x00007f4e07e904c2 in g_main_loop_run () from /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0
#7 0x000055e1b36ef725 in mainloop_run () at src/shared/mainloop-glib.c:79
#8 0x000055e1b36efb02 in mainloop_run_with_signal (func=<optimized out>, user_data=0x0) at src/shared/mainloop-notify.c:201
#9 0x000055e1b364b15e in main (argc=<optimized out>, argv=<optimized out>) at src/main.c:770
(gdb) print lsep
$1 = (struct avdtp_local_sep *) 0x0
On Sunday 26 April 2020 17:04:35 Pali Rohár wrote:
> Hello!
>
> Bluez bluetoothd daemon compiled from git master branch crashes when I
> try to call DBus method for switching A2DP codec. Below is stacktrace
> from gdb. It looks like NULL pointer dereference. It is reproducible.
>
> Program received signal SIGSEGV, Segmentation fault.
> 0x000055e1b3659c1a in avdtp_find_remote_sep (session=0x55e1b408bf80, lsep=0x0) at profiles/audio/avdtp.c:1221
> 1221 if (lsep->info.inuse)
> (gdb) bt
> #0 0x000055e1b3659c1a in avdtp_find_remote_sep (session=0x55e1b408bf80, lsep=0x0) at profiles/audio/avdtp.c:1221
> #1 0x000055e1b36568fc in find_remote_sep (sep=<optimized out>, chan=<optimized out>, chan=<optimized out>) at profiles/audio/a2dp.c:1169
> #2 0x000055e1b3656955 in a2dp_reconfigure (data=0x55e1b40a1e10) at profiles/audio/a2dp.c:1188
> #3 0x00007f4e07e90863 in ?? () from /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0
> #4 0x00007f4e07e8fdd8 in g_main_context_dispatch () from /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0
> #5 0x00007f4e07e901c8 in ?? () from /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0
> #6 0x00007f4e07e904c2 in g_main_loop_run () from /usr/lib/x86_64-linux-gnu/libglib-2.0.so.0
> #7 0x000055e1b36ef725 in mainloop_run () at src/shared/mainloop-glib.c:79
> #8 0x000055e1b36efb02 in mainloop_run_with_signal (func=<optimized out>, user_data=0x0) at src/shared/mainloop-notify.c:201
> #9 0x000055e1b364b15e in main (argc=<optimized out>, argv=<optimized out>) at src/main.c:770
> (gdb) print lsep
> $1 = (struct avdtp_local_sep *) 0x0
It always happens if I kill target application (pulseaudio) during
bluetooth daemon is connecting to remote bluetooth headset. I guess that
there is a race condition between unregistering application agent
(together with unregistering all its local seps) and trying to use /
choose local sep for a new remote connection.
Here is simple patch which prevent bluetooth daemon crash:
diff --git a/profiles/audio/a2dp.c b/profiles/audio/a2dp.c
index a5590b24c..2f0fcd974 100644
--- a/profiles/audio/a2dp.c
+++ b/profiles/audio/a2dp.c
@@ -1184,8 +1184,14 @@ static gboolean a2dp_reconfigure(gpointer data)
rsep_codec = (struct avdtp_media_codec_capability *) cap->data;
}
- if (!setup->rsep || sep->codec != rsep_codec->media_codec_type)
+ if (!setup->rsep || sep->codec != rsep_codec->media_codec_type) {
+ if (!sep->lsep) {
+ error("no lsep");
+ posix_err = -EINVAL;
+ goto failed;
+ }
setup->rsep = find_remote_sep(setup->chan, sep);
+ }
posix_err = avdtp_set_configuration(setup->session, setup->rsep->sep,
sep->lsep,
After applying this patch I get following error message without any
crash in bluetooth log:
bluetoothd[...]: Error on avdtp_open Invalid argument (22)
Which is probably OK, as target application is not running anymore and
connect request could not be finished.
a2dp_reconfigure() is called as callback when local and remote SEP does not
have to be valid anymore, sep->lsep can be NULL.
This change fixes bluetoothd daemon crash (dereferencing NULL sep->lsep)
when audio agent disconnect in the middle of the reconfigure call.
---
profiles/audio/a2dp.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/profiles/audio/a2dp.c b/profiles/audio/a2dp.c
index a5590b24c..8e6d8b417 100644
--- a/profiles/audio/a2dp.c
+++ b/profiles/audio/a2dp.c
@@ -1179,6 +1179,12 @@ static gboolean a2dp_reconfigure(gpointer data)
struct avdtp_media_codec_capability *rsep_codec;
struct avdtp_service_capability *cap;
+ if (!sep->lsep) {
+ error("a2dp_reconfigure: no valid local SEP");
+ posix_err = -EINVAL;
+ goto failed;
+ }
+
if (setup->rsep) {
cap = avdtp_get_codec(setup->rsep->sep);
rsep_codec = (struct avdtp_media_codec_capability *) cap->data;
@@ -1187,6 +1193,12 @@ static gboolean a2dp_reconfigure(gpointer data)
if (!setup->rsep || sep->codec != rsep_codec->media_codec_type)
setup->rsep = find_remote_sep(setup->chan, sep);
+ if (!setup->rsep) {
+ error("a2dp_reconfigure: unable to find remote SEP");
+ posix_err = -EINVAL;
+ goto failed;
+ }
+
posix_err = avdtp_set_configuration(setup->session, setup->rsep->sep,
sep->lsep,
setup->caps,
--
2.20.1
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.
While we are preparing for reviewing the patches, we found the following
issue/warning.
Test Result:
Checkpatch Failed
Patch Title:
a2dp: Check for valid SEP in a2dp_reconfigure
Output:
WARNING:EMBEDDED_FUNCTION_NAME: Prefer using '"%s...", __func__' to using 'a2dp_reconfigure', this function's name, in a string
#21: FILE: profiles/audio/a2dp.c:1183:
+ error("a2dp_reconfigure: no valid local SEP");
WARNING:EMBEDDED_FUNCTION_NAME: Prefer using '"%s...", __func__' to using 'a2dp_reconfigure', this function's name, in a string
#34: FILE: profiles/audio/a2dp.c:1197:
+ error("a2dp_reconfigure: unable to find remote SEP");
- total: 0 errors, 2 warnings, 24 lines checked
NOTE: For some of the reported defects, checkpatch may be able to
mechanically convert to the typical style using --fix or --fix-inplace.
Your patch has style problems, please review.
NOTE: Ignored message types: COMMIT_MESSAGE COMPLEX_MACRO CONST_STRUCT FILE_PATH_CHANGES MISSING_SIGN_OFF PREFER_PACKED SPLIT_STRING
NOTE: If any of the errors are false positives, please report
them to the maintainer, see CHECKPATCH in MAINTAINERS.
For more details about BlueZ coding style guide, please find it
in doc/coding-style.txt
---
Regards,
Linux Bluetooth
a2dp_reconfigure() is called as callback when local and remote SEP does not
have to be valid anymore, sep->lsep can be NULL.
This change fixes bluetoothd daemon crash (dereferencing NULL sep->lsep)
when audio agent disconnect in the middle of the reconfigure call.
---
profiles/audio/a2dp.c | 12 ++++++++++++
1 file changed, 12 insertions(+)
diff --git a/profiles/audio/a2dp.c b/profiles/audio/a2dp.c
index c31aaf187..a2ce3204d 100644
--- a/profiles/audio/a2dp.c
+++ b/profiles/audio/a2dp.c
@@ -1178,6 +1178,12 @@ static gboolean a2dp_reconfigure(gpointer data)
struct avdtp_media_codec_capability *rsep_codec;
struct avdtp_service_capability *cap;
+ if (!sep->lsep) {
+ error("no valid local SEP");
+ posix_err = -EINVAL;
+ goto failed;
+ }
+
if (setup->rsep) {
cap = avdtp_get_codec(setup->rsep->sep);
rsep_codec = (struct avdtp_media_codec_capability *) cap->data;
@@ -1186,6 +1192,12 @@ static gboolean a2dp_reconfigure(gpointer data)
if (!setup->rsep || sep->codec != rsep_codec->media_codec_type)
setup->rsep = find_remote_sep(setup->chan, sep);
+ if (!setup->rsep) {
+ error("unable to find remote SEP");
+ posix_err = -EINVAL;
+ goto failed;
+ }
+
posix_err = avdtp_set_configuration(setup->session, setup->rsep->sep,
sep->lsep,
setup->caps,
--
2.20.1
Hi Pali,
On Sun, May 3, 2020 at 4:06 AM Pali Rohár <[email protected]> wrote:
>
> a2dp_reconfigure() is called as callback when local and remote SEP does not
> have to be valid anymore, sep->lsep can be NULL.
>
> This change fixes bluetoothd daemon crash (dereferencing NULL sep->lsep)
> when audio agent disconnect in the middle of the reconfigure call.
> ---
> profiles/audio/a2dp.c | 12 ++++++++++++
> 1 file changed, 12 insertions(+)
>
> diff --git a/profiles/audio/a2dp.c b/profiles/audio/a2dp.c
> index c31aaf187..a2ce3204d 100644
> --- a/profiles/audio/a2dp.c
> +++ b/profiles/audio/a2dp.c
> @@ -1178,6 +1178,12 @@ static gboolean a2dp_reconfigure(gpointer data)
> struct avdtp_media_codec_capability *rsep_codec;
> struct avdtp_service_capability *cap;
>
> + if (!sep->lsep) {
> + error("no valid local SEP");
> + posix_err = -EINVAL;
> + goto failed;
> + }
> +
> if (setup->rsep) {
> cap = avdtp_get_codec(setup->rsep->sep);
> rsep_codec = (struct avdtp_media_codec_capability *) cap->data;
> @@ -1186,6 +1192,12 @@ static gboolean a2dp_reconfigure(gpointer data)
> if (!setup->rsep || sep->codec != rsep_codec->media_codec_type)
> setup->rsep = find_remote_sep(setup->chan, sep);
>
> + if (!setup->rsep) {
> + error("unable to find remote SEP");
> + posix_err = -EINVAL;
> + goto failed;
> + }
> +
> posix_err = avdtp_set_configuration(setup->session, setup->rsep->sep,
> sep->lsep,
> setup->caps,
> --
> 2.20.1
Applied, thanks.
--
Luiz Augusto von Dentz