diff mbox series

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

Message ID 20201102142933.BlueZ.v2.1.I474ca6a46b5ae198462df03d23f46dee652f74bb@changeid (mailing list archive)
State Accepted
Delegated to: Luiz Von Dentz
Headers show
Series [BlueZ,v2] adapter: Fix a crash caused by lingering discovery client pointer | expand

Commit Message

Miao-chen Chou Nov. 2, 2020, 10:30 p.m. UTC
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 <alainm@chromium.org>
Reviewed-by: Sonny Sasaka <sonnysasaka@chromium.org>
---

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(-)

Comments

Luiz Augusto von Dentz Nov. 2, 2020, 11:28 p.m. UTC | #1
Hi Miao,

On Mon, Nov 2, 2020 at 2:36 PM Miao-chen Chou <mcchou@chromium.org> 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 <alainm@chromium.org>
> Reviewed-by: Sonny Sasaka <sonnysasaka@chromium.org>
> ---
>
> 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.
bluez.test.bot@gmail.com Nov. 4, 2020, 7:22 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=375919

---Test result---

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

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

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

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



---
Regards,
Linux Bluetooth
diff mbox series

Patch

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);