2020-11-02 22:34:05

by Miao-chen Chou

Subject: [BlueZ PATCH v2] adapter: Fix a crash caused by lingering discovery client pointer

This cleans up the lingering pointer, adapter->client, during powering
off the adapter. The crash occurs when a D-Bus client set Powered
property to false and immediately calls StopDiscovery() when there is
ongoing discovery. As a part of powering off the adapter,
adapter->discovery_list gets cleared, and given that adapter->client
refers to one of the clients in adapter->discovery_list, adapter->client
should be cleared along with it.

(1) Connect to a BT audio device from BT system tray.
(2) Once the audio device is connected, power off BT and immediately
power off the audio device.

Reviewed-by: Alain Michaud <[email protected]>
Reviewed-by: Sonny Sasaka <[email protected]>
---

Changes in v2:
- Move the D-Bus method call clean-up to discovery_free()

src/adapter.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)

diff --git a/src/adapter.c b/src/adapter.c
index c0053000a..f02ab799d 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -1496,6 +1496,7 @@ static void discovery_cleanup(struct btd_adapter *adapter, int timeout)
static void discovery_free(void *user_data)
{
struct discovery_client *client = user_data;
+ struct btd_adapter *adapter = client->adapter;

DBG("%p", client);

@@ -1507,8 +1508,14 @@ static void discovery_free(void *user_data)
client->discovery_filter = NULL;
}

- if (client->msg)
+ if (client->msg) {
+ if (client == adapter->client) {
+ g_dbus_send_message(dbus_conn,
+ btd_error_busy(client->msg));
+ adapter->client = NULL;
+ }
dbus_message_unref(client->msg);
+ }

g_free(client->owner);
g_free(client);
--
2.26.2

2020-11-02 23:29:26

by Luiz Augusto von Dentz

Subject: Re: [BlueZ PATCH v2] adapter: Fix a crash caused by lingering discovery client pointer

Hi Miao,

On Mon, Nov 2, 2020 at 2:36 PM Miao-chen Chou <[email protected]> wrote:
>
> This cleans up the lingering pointer, adapter->client, during powering
> off the adapter. The crash occurs when a D-Bus client set Powered
> property to false and immediately calls StopDiscovery() when there is
> ongoing discovery. As a part of powering off the adapter,
> adapter->discovery_list gets cleared, and given that adapter->client
> refers to one of the clients in adapter->discovery_list, adapter->client
> should be cleared along with it.
>
> (1) Connect to a BT audio device from BT system tray.
> (2) Once the audio device is connected, power off BT and immediately
> power off the audio device.
>
> Reviewed-by: Alain Michaud <[email protected]>
> Reviewed-by: Sonny Sasaka <[email protected]>
> ---
>
> Changes in v2:
> - Move the D-Bus method call clean-up to discovery_free()
>
> src/adapter.c | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/src/adapter.c b/src/adapter.c
> index c0053000a..f02ab799d 100644
> --- a/src/adapter.c
> +++ b/src/adapter.c
> @@ -1496,6 +1496,7 @@ static void discovery_cleanup(struct btd_adapter *adapter, int timeout)
> static void discovery_free(void *user_data)
> {
> struct discovery_client *client = user_data;
> + struct btd_adapter *adapter = client->adapter;
>
> DBG("%p", client);
>
> @@ -1507,8 +1508,14 @@ static void discovery_free(void *user_data)
> client->discovery_filter = NULL;
> }
>
> - if (client->msg)
> + if (client->msg) {
> + if (client == adapter->client) {
> + g_dbus_send_message(dbus_conn,
> + btd_error_busy(client->msg));
> + adapter->client = NULL;
> + }
> dbus_message_unref(client->msg);
> + }
>
> g_free(client->owner);
> g_free(client);
> --
> 2.26.2

Applied, thanks.

--
Luiz Augusto von Dentz

2020-11-04 19:32:09

by bluez.test.bot

Subject: RE: [BlueZ,v2] adapter: Fix a crash caused by lingering discovery client pointer

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=375919

---Test result---

##############################
Test: CheckPatch - PASS

##############################
Test: CheckGitLint - PASS

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

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



---
Regards,
Linux Bluetooth