diff mbox series

[BlueZ] Cancel GATT client operations when cleaning up GATT cache

Message ID 20201105224923.377-1-sonnysasaka@chromium.org
State New
Headers show
Series [BlueZ] Cancel GATT client operations when cleaning up GATT cache | expand

Commit Message

Sonny Sasaka Nov. 5, 2020, 10:49 p.m. UTC 
A crash is found when discovery_complete_op is invoked but
gatt_db_attribute objects have been freed. A solution is to always
cancel GATT client operations every time we clean GATT cache.

The crash is reproducible by connecting to an LE mouse and then calling
RemoveDevice immediately, triggering disconnection while GATT service
discovery is in progress.

Sample crash stack trace:
0  gatt_db_service_get_handles (service=0x1751130, service=0x1751130, end_handle=0x7ffcd600806e, start_handle=0x7ffcd600806c) at src/shared/gatt-db.c:569
1  gatt_db_attribute_get_service_data (attrib=<optimized out>, start_handle=0x7ffcd600806c, end_handle=0x7ffcd600806e, primary=0x0, uuid=0x0) at src/shared/gatt-db.c:1657
2  0x00000000004983a8 in discovery_op_complete (op=op@entry=0x173b320, success=<optimized out>, err=err@entry=10 '\n') at src/shared/gatt-client.c:406
3  0x000000000049a548 in discover_descs_cb (success=<optimized out>, att_ecode=<optimized out>, result=<optimized out>, user_data=0x173b320) at src/shared/gatt-client.c:915
4  0x00000000004a1d87 in discovery_op_complete (op=0x1748450, success=<optimized out>, ecode=<optimized out>) at src/shared/gatt-helpers.c:615
5  0x00000000004a2379 in discover_descs_cb (opcode=<optimized out>, pdu=0x174d551, length=<optimized out>, user_data=0x1748450) at src/shared/gatt-helpers.c:1465
6  0x00000000004966db in handle_rsp (pdu_len=4, pdu=<optimized out>, opcode=<optimized out>, chan=0x17483c0) at src/shared/att.c:814
7  can_read_data (io=<optimized out>, user_data=0x17483c0) at src/shared/att.c:1011
8  0x00000000004a0853 in watch_callback (channel=<optimized out>, cond=<optimized out>, user_data=<optimized out>) at src/shared/io-glib.c:157
9  0x00007fb3f2d7fe87 in g_main_context_dispatch () from /usr/lib64/libglib-2.0.so.0
10 0x00007fb3f2d80230 in ?? () from /usr/lib64/libglib-2.0.so.0
11 0x00007fb3f2d80542 in g_main_loop_run () from /usr/lib64/libglib-2.0.so.0
12 0x00000000004a0e25 in mainloop_run () at src/shared/mainloop-glib.c:66
13 0x00000000004a11f2 in mainloop_run_with_signal (func=func@entry=0x43f200 <signal_callback>, user_data=user_data@entry=0x0) at src/shared/mainloop-notify.c:188
14 0x000000000040c72e in main (argc=<optimized out>, argv=<optimized out>) at src/main.c:959

Change-Id: I17b8ccc5322b0a83fc63d711e83c9f4a58a0374c
---
 src/device.c | 1 +
 1 file changed, 1 insertion(+)

Comments

Luiz Augusto von Dentz Nov. 5, 2020, 11:22 p.m. UTC | #1 
Hi Sonny,

On Thu, Nov 5, 2020 at 2:53 PM Sonny Sasaka <sonnysasaka@chromium.org> wrote:
>

> A crash is found when discovery_complete_op is invoked but

> gatt_db_attribute objects have been freed. A solution is to always

> cancel GATT client operations every time we clean GATT cache.

>

> The crash is reproducible by connecting to an LE mouse and then calling

> RemoveDevice immediately, triggering disconnection while GATT service

> discovery is in progress.

>

> Sample crash stack trace:

> 0  gatt_db_service_get_handles (service=0x1751130, service=0x1751130, end_handle=0x7ffcd600806e, start_handle=0x7ffcd600806c) at src/shared/gatt-db.c:569

> 1  gatt_db_attribute_get_service_data (attrib=<optimized out>, start_handle=0x7ffcd600806c, end_handle=0x7ffcd600806e, primary=0x0, uuid=0x0) at src/shared/gatt-db.c:1657

> 2  0x00000000004983a8 in discovery_op_complete (op=op@entry=0x173b320, success=<optimized out>, err=err@entry=10 '\n') at src/shared/gatt-client.c:406

> 3  0x000000000049a548 in discover_descs_cb (success=<optimized out>, att_ecode=<optimized out>, result=<optimized out>, user_data=0x173b320) at src/shared/gatt-client.c:915

> 4  0x00000000004a1d87 in discovery_op_complete (op=0x1748450, success=<optimized out>, ecode=<optimized out>) at src/shared/gatt-helpers.c:615

> 5  0x00000000004a2379 in discover_descs_cb (opcode=<optimized out>, pdu=0x174d551, length=<optimized out>, user_data=0x1748450) at src/shared/gatt-helpers.c:1465

> 6  0x00000000004966db in handle_rsp (pdu_len=4, pdu=<optimized out>, opcode=<optimized out>, chan=0x17483c0) at src/shared/att.c:814

> 7  can_read_data (io=<optimized out>, user_data=0x17483c0) at src/shared/att.c:1011

> 8  0x00000000004a0853 in watch_callback (channel=<optimized out>, cond=<optimized out>, user_data=<optimized out>) at src/shared/io-glib.c:157

> 9  0x00007fb3f2d7fe87 in g_main_context_dispatch () from /usr/lib64/libglib-2.0.so.0

> 10 0x00007fb3f2d80230 in ?? () from /usr/lib64/libglib-2.0.so.0

> 11 0x00007fb3f2d80542 in g_main_loop_run () from /usr/lib64/libglib-2.0.so.0

> 12 0x00000000004a0e25 in mainloop_run () at src/shared/mainloop-glib.c:66

> 13 0x00000000004a11f2 in mainloop_run_with_signal (func=func@entry=0x43f200 <signal_callback>, user_data=user_data@entry=0x0) at src/shared/mainloop-notify.c:188

> 14 0x000000000040c72e in main (argc=<optimized out>, argv=<optimized out>) at src/main.c:959

>

> Change-Id: I17b8ccc5322b0a83fc63d711e83c9f4a58a0374c

> ---

>  src/device.c | 1 +

>  1 file changed, 1 insertion(+)

>

> diff --git a/src/device.c b/src/device.c

> index a5ef46730..d5e884ff4 100644

> --- a/src/device.c

> +++ b/src/device.c

> @@ -569,6 +569,7 @@ static void gatt_cache_cleanup(struct btd_device *device)

>         if (gatt_cache_is_enabled(device))

>                 return;

>

> +       bt_gatt_client_cancel_all(device->client);

>         gatt_db_clear(device->db);

>  }

>

> --

> 2.26.2


Applied, thanks.

-- 
Luiz Augusto von Dentz
bluez.test.bot@gmail.com Nov. 5, 2020, 11:24 p.m. UTC | #2 
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=378543

---Test result---

##############################
Test: CheckPatch - FAIL
Output:
Cancel GATT client operations when cleaning up GATT cache
WARNING:COMMIT_LOG_LONG_LINE: Possible unwrapped commit description (prefer a maximum 75 chars per line)
#15: 
0  gatt_db_service_get_handles (service=0x1751130, service=0x1751130, end_handle=0x7ffcd600806e, start_handle=0x7ffcd600806c) at src/shared/gatt-db.c:569

ERROR:GERRIT_CHANGE_ID: Remove Gerrit Change-Id's before submitting upstream
#31: 
Change-Id: I17b8ccc5322b0a83fc63d711e83c9f4a58a0374c

- total: 1 errors, 1 warnings, 7 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.

"[PATCH] Cancel GATT client operations when cleaning up GATT cache" 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 SSCANF_TO_KSTRTO

NOTE: If any of the errors are false positives, please report
      them to the maintainer, see CHECKPATCH in MAINTAINERS.


##############################
Test: CheckGitLint - FAIL
Output:
Cancel GATT client operations when cleaning up GATT cache
12: B1 Line exceeds max length (153>80): "0  gatt_db_service_get_handles (service=0x1751130, service=0x1751130, end_handle=0x7ffcd600806e, start_handle=0x7ffcd600806c) at src/shared/gatt-db.c:569"
13: B1 Line exceeds max length (170>80): "1  gatt_db_attribute_get_service_data (attrib=<optimized out>, start_handle=0x7ffcd600806c, end_handle=0x7ffcd600806e, primary=0x0, uuid=0x0) at src/shared/gatt-db.c:1657"
14: B1 Line exceeds max length (150>80): "2  0x00000000004983a8 in discovery_op_complete (op=op@entry=0x173b320, success=<optimized out>, err=err@entry=10 '\n') at src/shared/gatt-client.c:406"
15: B1 Line exceeds max length (172>80): "3  0x000000000049a548 in discover_descs_cb (success=<optimized out>, att_ecode=<optimized out>, result=<optimized out>, user_data=0x173b320) at src/shared/gatt-client.c:915"
16: B1 Line exceeds max length (142>80): "4  0x00000000004a1d87 in discovery_op_complete (op=0x1748450, success=<optimized out>, ecode=<optimized out>) at src/shared/gatt-helpers.c:615"
17: B1 Line exceeds max length (161>80): "5  0x00000000004a2379 in discover_descs_cb (opcode=<optimized out>, pdu=0x174d551, length=<optimized out>, user_data=0x1748450) at src/shared/gatt-helpers.c:1465"
18: B1 Line exceeds max length (132>80): "6  0x00000000004966db in handle_rsp (pdu_len=4, pdu=<optimized out>, opcode=<optimized out>, chan=0x17483c0) at src/shared/att.c:814"
19: B1 Line exceeds max length (83>80): "7  can_read_data (io=<optimized out>, user_data=0x17483c0) at src/shared/att.c:1011"
20: B1 Line exceeds max length (142>80): "8  0x00000000004a0853 in watch_callback (channel=<optimized out>, cond=<optimized out>, user_data=<optimized out>) at src/shared/io-glib.c:157"
21: B1 Line exceeds max length (84>80): "9  0x00007fb3f2d7fe87 in g_main_context_dispatch () from /usr/lib64/libglib-2.0.so.0"
25: B1 Line exceeds max length (161>80): "13 0x00000000004a11f2 in mainloop_run_with_signal (func=func@entry=0x43f200 <signal_callback>, user_data=user_data@entry=0x0) at src/shared/mainloop-notify.c:188"
26: B1 Line exceeds max length (92>80): "14 0x000000000040c72e in main (argc=<optimized out>, argv=<optimized out>) at src/main.c:959"


##############################
Test: CheckBuild - PASS

##############################
Test: MakeCheck - PASS



---
Regards,
Linux Bluetooth
diff mbox series

Patch

diff --git a/src/device.c b/src/device.c
index a5ef46730..d5e884ff4 100644
--- a/src/device.c
+++ b/src/device.c
@@ -569,6 +569,7 @@  static void gatt_cache_cleanup(struct btd_device *device)
 	if (gatt_cache_is_enabled(device))
 		return;
 
+	bt_gatt_client_cancel_all(device->client);
 	gatt_db_clear(device->db);
 }