Received-SPF: pass (google.com: domain of linux-bluetooth-owner@vger.kernel.org designates 23.128.96.18 as permitted sender) client-ip=23.128.96.18;
MIME-Version: 1.0
References: <20201105224923.377-1-sonnysasaka@chromium.org>
In-Reply-To: <20201105224923.377-1-sonnysasaka@chromium.org>
From:   Luiz Augusto von Dentz <luiz.dentz@gmail.com>
Date:   Thu, 5 Nov 2020 15:22:00 -0800
Message-ID: <CABBYNZ+WHENGaL9NGD0F8bf6zaw9Omgb8iczRqWRcVWUAYY7Xw@mail.gmail.com>
Subject: Re: [PATCH BlueZ] Cancel GATT client operations when cleaning up GATT cache
To:     Sonny Sasaka <sonnysasaka@chromium.org>
Cc:     "linux-bluetooth@vger.kernel.org" <linux-bluetooth@vger.kernel.org>
Content-Type: text/plain; charset="UTF-8"
Precedence: bulk

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