Apple Airpods disconnect causing the crash #533

ahmedShafi12 · 2023-06-14T13:13:31Z

Disconnect the Apple Airpods. Remove one of the headphone from left ear and put in the Air pods case and close it is causing the crash
Call stack
setup_unref
media_endpoint_cancel
clear_endpoint
avdtp_sep_set_state
connection_lost
session_cb
main

2023 May 03 14:18:10.042481 bluetoothd[27482]: ../bluez-5.48/profiles/audio/a2dp.c:setup_free() 0x8f2ed8
2023 May 03 14:18:10.043061 bluetoothd[27482]: ../bluez-5.48/profiles/audio/avdtp.c:avdtp_unref() 0x9375a8: ref=1
2023 May 03 14:18:10.043549 bluetoothd[27482]: ../bluez-5.48/profiles/audio/a2dp.c:setup_unref() 0x8f2ed8: ref=-1
2023 May 03 14:18:10.055367 bluetoothd[27482]: ../bluez-5.48/profiles/audio/a2dp.c:setup_free() 0x8f2ed8
2023 May 03 14:18:10.055822 bluetoothd[27482]: ../bluez-5.48/profiles/audio/avdtp.c:avdtp_unref() 0xb3ddf608: ref=9573823

The crash is due to ref is being reduce to negative number and then Setupfree is deleting the g_slist_free_full which is already free

Because the check is , Ref greater than 0.

if (setup->ref > 0)
return;
setup_free(setup);

I have added fix and resolved the issue . Can you please review the changes
diff -Naur bluez-5.48.orig/profiles/audio/a2dp.c bluez-5.48/profiles/audio/a2dp.c
--- bluez-5.48.orig/profiles/audio/a2dp.c 2023-06-08 15:24:07.658122948 +0000
+++ bluez-5.48/profiles/audio/a2dp.c 2023-06-08 15:35:50.387580121 +0000
@@ -178,14 +178,15 @@

static void setup_unref(struct a2dp_setup *setup)
{

setup->ref--;

```
   setup->ref--;
```

DBG("%p: ref=%d", setup, setup->ref);

   DBG("%p: ref=%d", setup, setup->ref);

if (setup->ref > 0)
```
  return;
```

```
   if (setup->ref == 0)
```
```
           setup_free(setup);
```
```
   else
```
```
           return;
```

setup_free(setup);
}

static struct a2dp_setup_cb *setup_cb_new(struct a2dp_setup *setup)
diff -Naur bluez-5.48.orig/profiles/audio/avdtp.c bluez-5.48/profiles/audio/avdtp.c
--- bluez-5.48.orig/profiles/audio/avdtp.c 2023-06-08 15:24:07.662123022 +0000
+++ bluez-5.48/profiles/audio/avdtp.c 2023-06-08 15:42:34.351257198 +0000
@@ -1180,27 +1180,26 @@

DBG("%p: ref=%d", session, session->ref);

if (session->ref > 0)
```
  return;
```
switch (session->state) {
case AVDTP_SESSION_STATE_CONNECTED:

```
   if (session->ref == 0) {
```
```
         switch (session->state) {
```

             case AVDTP_SESSION_STATE_CONNECTED:
  /* Only set disconnect timer if there are local endpoints
   * otherwise disconnect immediately.
   */

```
  if (queue_isempty(session->lseps))
```

  	connection_lost(session, ECONNRESET);

```
  else
```
```
  	set_disconnect_timer(session);
```
```
  break;
```
case AVDTP_SESSION_STATE_CONNECTING:

  connection_lost(session, ECONNABORTED);

```
  break;
```
case AVDTP_SESSION_STATE_DISCONNECTED:
default:
```
  avdtp_free(session);
```
```
  break;
```
}

            if (queue_isempty(session->lseps))

                  connection_lost(session, ECONNRESET);

```
            else
```

                  set_disconnect_timer(session);

```
            break;
```

            case AVDTP_SESSION_STATE_CONNECTING:

                  connection_lost(session, ECONNABORTED);

```
            break;
```

            case AVDTP_SESSION_STATE_DISCONNECTED:

```
            default:
```
```
                 avdtp_free(session);
```
```
            break;
```
```
         }
```
```
   }
```

}

struct avdtp *avdtp_ref(struct avdtp *session)

The text was updated successfully, but these errors were encountered:

joakim-tjernlund · 2023-06-14T14:01:30Z

Please use ``` to quote code, it is not readable now

joakim-tjernlund · 2023-06-14T14:04:05Z

Is this really based on bluez-5.48? If so can you rebase/check master?

joakim-tjernlund · 2023-06-14T14:07:13Z

Any chance this is same/similar to #532 ?

ahmedShafi12 · 2023-06-14T14:13:18Z

bluez-5.48-039-bluetooth_a2dp_ref_negative_abort.patch

I have attached the patch as file . NO it does not look alike #532

"""0‎:1raise‎/usr/src/debug/glibc/2.31+gitAUTOINC+1094741224-r0/git/signal/../sysdeps/unix/sysv/linux/internal-signals.h:862abort‎/usr/src/debug/glibc/2.31+gitAUTOINC+1094741224-r0/git/stdlib/abort.c:793__libc_message‎/usr/src/debug/glibc/2.31+gitAUTOINC+1094741224-r0/git/libio/../sysdeps/posix/libc_fatal.c:1554malloc_printerr‎/usr/src/debug/glibc/2.31+gitAUTOINC+1094741224-r0/git/malloc/malloc.c:53475_int_free‎/usr/src/debug/glibc/2.31+gitAUTOINC+1094741224-r0/git/malloc/malloc.c:41776g_slist_foreach‎/usr/src/debug/glib-2.0/1_2.62.4-r0/build/../glib-2.62.4/glib/gslist.c:8647g_slist_free_full‎/usr/src/debug/glib-2.0/1_2.62.4-r0/build/../glib-2.62.4/glib/gslist.c:1778setup_unref‎/usr/src/debug/bluez5/5.48-r0/build/../bluez-5.48/profiles/audio/a2dp.c:1759media_endpoint_cancel‎/usr/src/debug/bluez5/5.48-r0/build/../bluez-5.48/profiles/audio/media.c:14210clear_endpoint‎/usr/src/debug/bluez5/5.48-r0/build/../bluez-5.48/profiles/audio/media.c:15011avdtp_sep_set_state‎/usr/src/debug/bluez5/5.48-r0/build/../bluez-5.48/profiles/audio/avdtp.c:101712g_slist_foreach‎/usr/src/debug/glib-2.0/1_2.62.4-r0/build/../glib-2.62.4/glib/gslist.c:86413connection_lost‎/usr/src/debug/bluez5/5.48-r0/build/../bluez-5.48/profiles/audio/avdtp.c:111714session_cb‎/usr/src/debug/bluez5/5.48-r0/build/../bluez-5.48/profiles/audio/avdtp.c:226315g_main_context_dispatch‎/usr/src/debug/glib-2.0/1_2.62.4-r0/build/../glib-2.62.4/glib/gmain.c:321616g_main_context_iterate‎/usr/src/debug/glib-2.0/1_2.62.4-r0/build/../glib-2.62.4/glib/gmain.c:395817g_main_loop_run‎/usr/src/debug/glib-2.0/1_2.62.4-r0/build/../glib-2.62.4/glib/gmain.c:415218main‎/usr/src/debug/bluez5/5.48-r0/build/../bluez-5.48/src/main.c:90119__libc_start_main‎/usr/src/debug/glibc/2.31+gitAUTOINC+1094741224-r0/git/csu/libc-start.c:30820‎:21__libc_csu_init‎/usr/src/debug/glibc/2.31+gitAUTOINC+1094741224-r0/git/csu/elf-init.c:8722‎:23‎:STACK CONTENTOS|Linux|0.0.0 Linux 4.9.113 #1 SMP PREEMPT Mon May 1 17:48:06 UTC 2023 armv7l
CPU|arm|ARMv1 ARM part(0x4100d050) features: half,thumb,fastmult,vfpv2,edsp,neon,vfpv3,tls,vfpv4,idiva,idivt|4
GPU|||
Crash|SIGABRT|0x523c|0
0
‎:
1
raise
‎/usr/src/debug/glibc/2.31+gitAUTOINC+1094741224-r0/git/signal/../sysdeps/unix/sysv/linux/internal-signals.h:86
2
abort
‎/usr/src/debug/glibc/2.31+gitAUTOINC+1094741224-r0/git/stdlib/abort.c:79
3
__libc_message
‎/usr/src/debug/glibc/2.31+gitAUTOINC+1094741224-r0/git/libio/../sysdeps/posix/libc_fatal.c:155
4
malloc_printerr
‎/usr/src/debug/glibc/2.31+gitAUTOINC+1094741224-r0/git/malloc/malloc.c:5347
5
_int_free
‎/usr/src/debug/glibc/2.31+gitAUTOINC+1094741224-r0/git/malloc/malloc.c:4177
6
g_slist_foreach
‎/usr/src/debug/glib-2.0/1_2.62.4-r0/build/../glib-2.62.4/glib/gslist.c:864
7
g_slist_free_full
‎/usr/src/debug/glib-2.0/1_2.62.4-r0/build/../glib-2.62.4/glib/gslist.c:177
8
setup_unref
‎/usr/src/debug/bluez5/5.48-r0/build/../bluez-5.48/profiles/audio/a2dp.c:175
9
media_endpoint_cancel
‎/usr/src/debug/bluez5/5.48-r0/build/../bluez-5.48/profiles/audio/media.c:142
10
clear_endpoint
‎/usr/src/debug/bluez5/5.48-r0/build/../bluez-5.48/profiles/audio/media.c:150
11
avdtp_sep_set_state
‎/usr/src/debug/bluez5/5.48-r0/build/../bluez-5.48/profiles/audio/avdtp.c:1017
12
g_slist_foreach
‎/usr/src/debug/glib-2.0/1_2.62.4-r0/build/../glib-2.62.4/glib/gslist.c:864
13
connection_lost
‎/usr/src/debug/bluez5/5.48-r0/build/../bluez-5.48/profiles/audio/avdtp.c:1117
14
session_cb
‎/usr/src/debug/bluez5/5.48-r0/build/../bluez-5.48/profiles/audio/avdtp.c:2263
15
g_main_context_dispatch
‎/usr/src/debug/glib-2.0/1_2.62.4-r0/build/../glib-2.62.4/glib/gmain.c:3216
16
g_main_context_iterate
‎/usr/src/debug/glib-2.0/1_2.62.4-r0/build/../glib-2.62.4/glib/gmain.c:3958
17
g_main_loop_run
‎/usr/src/debug/glib-2.0/1_2.62.4-r0/build/../glib-2.62.4/glib/gmain.c:4152
18
main
‎/usr/src/debug/bluez5/5.48-r0/build/../bluez-5.48/src/main.c:901
19
__libc_start_main
‎/usr/src/debug/glibc/2.31+gitAUTOINC+1094741224-r0/git/csu/libc-start.c:308
20
‎:
21
__libc_csu_init
‎/usr/src/debug/glibc/2.31+gitAUTOINC+1094741224-r0/git/csu/elf-init.c:87
22
‎:
23
‎:
ATTRIBUTES"""

joakim-tjernlund · 2023-06-14T14:19:25Z

I see, the patch is white space damaged though(change of SPACE/TAB only)

Too bad #532 looks unrelated ... Any ideas welcome

ahmedShafi12 · 2023-06-14T14:25:32Z

Basically the fix is the setup->ref should never be negative , As in this case while setup->ref is negative its failing the condition
"""if (setup->ref > 0)""
and execute
setup_free(setup); which try to delete already deleted memory

joakim-tjernlund · 2023-06-14T14:25:52Z

Look like you patche can be written:

--- a2dp.c.org	2023-06-14 16:20:45.777263502 +0200
+++ a2dp.c	2023-06-14 16:21:52.650843681 +0200
@@ -231,10 +231,8 @@
 
 	DBG("%p: ref=%d", setup, setup->ref);
 
-	if (setup->ref > 0)
-		return;
-
-	setup_free(setup);
+	if (setup->ref == 0)
+		setup_free(setup);
 }
 
 static struct a2dp_setup_cb *setup_cb_new(struct a2dp_setup *setup)

and

--- avdtp.c.org	2023-06-14 16:22:15.353699371 +0200
+++ avdtp.c	2023-06-14 16:24:05.250988808 +0200
@@ -1220,7 +1220,7 @@
 
 	DBG("%p: ref=%d", session, session->ref);
 
-	if (session->ref > 0)
+	if (session->ref != 0)
 		return;
 
 	switch (session->state) {

I cannot tell if this is the right fix

ahmedShafi12 · 2023-06-14T14:31:36Z

Thank you will try the above changes, it should work . One more thing
setup->ref is int so we get negative number and session->ref is unsigned int. its getting big number and how about changing the declaration to int and assign it zero if its negative number

joakim-tjernlund · 2023-06-14T14:41:07Z

Thank you will try the above changes, it should work . One more thing setup->ref is int so we get negative number and session->ref is unsigned int. its getting big number and how about changing the declaration to int and assign it zero if its negative number

I am thinking ref shoul not be allowed to be negative, just skipping ref-- if ref = 0 or some such

ahmedShafi12 · 2023-06-14T14:50:17Z

Yes below should also work
'''+ If( ref == 0)

```
 return ;'''
```

joakim-tjernlund · 2023-06-14T14:52:16Z

Use preview to check formatting

Vudentz · 2023-06-14T16:46:16Z

This looks like an unbalanced reference thing, there is one too many avdtp_unref it seems.

joakim-tjernlund · 2023-06-15T11:37:12Z

This looks like an unbalanced reference thing, there is one too many avdtp_unref it seems.

Yes, but the above defensive patch will hopefully not hard crash bluez when that happens so
I think it a good idea to add them. Also make unref symmetric to ref functions.

Same with the patch in #532

--- profiles/audio/transport.c.org	2023-06-13 18:38:14.542137531 +0200
+++ profiles/audio/transport.c	2023-06-13 18:39:22.110965184 +0200
@@ -345,7 +345,7 @@
 	struct media_owner *owner = user_data;
 	struct media_request *req = owner->pending;
 	struct media_transport *transport = owner->transport;
-	struct a2dp_sep *sep = media_endpoint_get_sep(transport->endpoint);
+	struct a2dp_sep *sep;
 	struct avdtp_stream *stream;
 	int fd;
 	uint16_t imtu, omtu;
@@ -356,6 +356,7 @@
 	if (err)
 		goto fail;
 
+	sep = media_endpoint_get_sep(transport->endpoint);
 	stream = a2dp_sep_get_stream(sep);
 	if (stream == NULL)
 		goto fail;

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Apple Airpods disconnect causing the crash #533

Apple Airpods disconnect causing the crash #533

ahmedShafi12 commented Jun 14, 2023

joakim-tjernlund commented Jun 14, 2023

joakim-tjernlund commented Jun 14, 2023

joakim-tjernlund commented Jun 14, 2023

ahmedShafi12 commented Jun 14, 2023

joakim-tjernlund commented Jun 14, 2023

ahmedShafi12 commented Jun 14, 2023

joakim-tjernlund commented Jun 14, 2023

ahmedShafi12 commented Jun 14, 2023

joakim-tjernlund commented Jun 14, 2023

ahmedShafi12 commented Jun 14, 2023 •

edited

joakim-tjernlund commented Jun 14, 2023

Vudentz commented Jun 14, 2023

joakim-tjernlund commented Jun 15, 2023

Apple Airpods disconnect causing the crash #533

Apple Airpods disconnect causing the crash #533

Comments

ahmedShafi12 commented Jun 14, 2023

joakim-tjernlund commented Jun 14, 2023

joakim-tjernlund commented Jun 14, 2023

joakim-tjernlund commented Jun 14, 2023

ahmedShafi12 commented Jun 14, 2023

joakim-tjernlund commented Jun 14, 2023

ahmedShafi12 commented Jun 14, 2023

joakim-tjernlund commented Jun 14, 2023

ahmedShafi12 commented Jun 14, 2023

joakim-tjernlund commented Jun 14, 2023

ahmedShafi12 commented Jun 14, 2023 • edited

joakim-tjernlund commented Jun 14, 2023

Vudentz commented Jun 14, 2023

joakim-tjernlund commented Jun 15, 2023

ahmedShafi12 commented Jun 14, 2023 •

edited