Re: [pulseaudio-discuss] [PATCH] bluetooth: Fix crash when disabling Bluetooth adapter

2019-07-02 Thread Tanu Kaskinen
On Wed, 2019-06-19 at 11:09 +0200, Frédéric Danis wrote:
> This crash occurs when PA is connected to a phone through the oFono
> backend.
> When disabling the Bluetooth adapter, pa_bluetooth_device is removed before
> hf_audio_card. Both keep refs on pa_bluetooth_transport. Those removal will
> call pa_bluetooth_transport_free() from device_free() (bluez5-util.c) and
> hf_audio_card_free() (backend-ofono.c).
> In the end, the call to pa_bluetooth_transport_free() calls
> pa_hasmap_remove() through pa_bluetooth_transport_unlink(), but since
> memory has already been freed, the second try results in a segfault.
> 
> Triggering hf_audio_card removal during pa_bluetooth_device removal allows
> hf_audio_card to be freed at the right time.
> ---
>  src/modules/bluetooth/backend-ofono.c | 18 ++
>  src/modules/bluetooth/bluez5-util.c   |  2 ++
>  src/modules/bluetooth/bluez5-util.h   |  1 +
>  3 files changed, 21 insertions(+)
> 
> diff --git a/src/modules/bluetooth/backend-ofono.c 
> b/src/modules/bluetooth/backend-ofono.c
> index 1f0efe923..0e5bbe8b7 100644
> --- a/src/modules/bluetooth/backend-ofono.c
> +++ b/src/modules/bluetooth/backend-ofono.c
> @@ -70,6 +70,7 @@ struct hf_audio_card {
>  int (*acquire)(struct hf_audio_card *card);
>  
>  pa_bluetooth_transport *transport;
> +pa_hook_slot *device_unlink_slot;
>  };
>  
>  struct pa_bluetooth_backend {
> @@ -181,6 +182,17 @@ static int card_acquire(struct hf_audio_card *card) {
>  return -1;
>  }
>  
> +static void hf_audio_agent_card_removed(pa_bluetooth_backend *backend, const 
> char *path);
> +
> +static pa_hook_result_t device_unlink_cb(pa_bluetooth_discovery *y, const 
> pa_bluetooth_device *d, struct hf_audio_card *card) {
> +pa_assert(d);
> +pa_assert(card);
> +
> +hf_audio_agent_card_removed(card->backend, card->path);
> +
> +return PA_HOOK_OK;
> +}
> +
>  static struct hf_audio_card *hf_audio_card_new(pa_bluetooth_backend 
> *backend, const char *path) {
>  struct hf_audio_card *card = pa_xnew0(struct hf_audio_card, 1);
>  
> @@ -189,12 +201,18 @@ static struct hf_audio_card 
> *hf_audio_card_new(pa_bluetooth_backend *backend, co
>  card->fd = -1;
>  card->acquire = card_acquire;
>  
> +card->device_unlink_slot = 
> pa_hook_connect(pa_bluetooth_discovery_hook(backend->discovery, 
> PA_BLUETOOTH_HOOK_DEVICE_UNLINK),
> +   PA_HOOK_NORMAL, 
> (pa_hook_cb_t) device_unlink_cb, card);
> +
>  return card;
>  }
>  
>  static void hf_audio_card_free(struct hf_audio_card *card) {
>  pa_assert(card);
>  
> +if (card->device_unlink_slot)
> +pa_hook_slot_free(card->device_unlink_slot);
> +
>  if (card->transport)
>  pa_bluetooth_transport_free(card->transport);
>  
> diff --git a/src/modules/bluetooth/bluez5-util.c 
> b/src/modules/bluetooth/bluez5-util.c
> index 3c5a000c0..d95c9c117 100644
> --- a/src/modules/bluetooth/bluez5-util.c
> +++ b/src/modules/bluetooth/bluez5-util.c
> @@ -562,6 +562,8 @@ static void device_free(pa_bluetooth_device *d) {
>  
>  device_stop_waiting_for_profiles(d);
>  
> +pa_hook_fire(>discovery->hooks[PA_BLUETOOTH_HOOK_DEVICE_UNLINK], d);
> +
>  for (i = 0; i < PA_BLUETOOTH_PROFILE_COUNT; i++) {
>  pa_bluetooth_transport *t;
>  
> diff --git a/src/modules/bluetooth/bluez5-util.h 
> b/src/modules/bluetooth/bluez5-util.h
> index 82739bffd..1e9caecb4 100644
> --- a/src/modules/bluetooth/bluez5-util.h
> +++ b/src/modules/bluetooth/bluez5-util.h
> @@ -50,6 +50,7 @@ typedef enum pa_bluetooth_hook {
>  PA_BLUETOOTH_HOOK_TRANSPORT_STATE_CHANGED,/* Call data: 
> pa_bluetooth_transport */
>  PA_BLUETOOTH_HOOK_TRANSPORT_MICROPHONE_GAIN_CHANGED,  /* Call data: 
> pa_bluetooth_transport */
>  PA_BLUETOOTH_HOOK_TRANSPORT_SPEAKER_GAIN_CHANGED, /* Call data: 
> pa_bluetooth_transport */
> +PA_BLUETOOTH_HOOK_DEVICE_UNLINK,  /* Call data: 
> pa_bluetooth_device */
>  PA_BLUETOOTH_HOOK_MAX
>  } pa_bluetooth_hook_t;
>  

Thanks! Applied.

-- 
Tanu

https://www.patreon.com/tanuk
https://liberapay.com/tanuk

___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss

[pulseaudio-discuss] [PATCH] bluetooth: Fix crash when disabling Bluetooth adapter

2019-06-19 Thread Frédéric Danis
This crash occurs when PA is connected to a phone through the oFono
backend.
When disabling the Bluetooth adapter, pa_bluetooth_device is removed before
hf_audio_card. Both keep refs on pa_bluetooth_transport. Those removal will
call pa_bluetooth_transport_free() from device_free() (bluez5-util.c) and
hf_audio_card_free() (backend-ofono.c).
In the end, the call to pa_bluetooth_transport_free() calls
pa_hasmap_remove() through pa_bluetooth_transport_unlink(), but since
memory has already been freed, the second try results in a segfault.

Triggering hf_audio_card removal during pa_bluetooth_device removal allows
hf_audio_card to be freed at the right time.
---
 src/modules/bluetooth/backend-ofono.c | 18 ++
 src/modules/bluetooth/bluez5-util.c   |  2 ++
 src/modules/bluetooth/bluez5-util.h   |  1 +
 3 files changed, 21 insertions(+)

diff --git a/src/modules/bluetooth/backend-ofono.c 
b/src/modules/bluetooth/backend-ofono.c
index 1f0efe923..0e5bbe8b7 100644
--- a/src/modules/bluetooth/backend-ofono.c
+++ b/src/modules/bluetooth/backend-ofono.c
@@ -70,6 +70,7 @@ struct hf_audio_card {
 int (*acquire)(struct hf_audio_card *card);
 
 pa_bluetooth_transport *transport;
+pa_hook_slot *device_unlink_slot;
 };
 
 struct pa_bluetooth_backend {
@@ -181,6 +182,17 @@ static int card_acquire(struct hf_audio_card *card) {
 return -1;
 }
 
+static void hf_audio_agent_card_removed(pa_bluetooth_backend *backend, const 
char *path);
+
+static pa_hook_result_t device_unlink_cb(pa_bluetooth_discovery *y, const 
pa_bluetooth_device *d, struct hf_audio_card *card) {
+pa_assert(d);
+pa_assert(card);
+
+hf_audio_agent_card_removed(card->backend, card->path);
+
+return PA_HOOK_OK;
+}
+
 static struct hf_audio_card *hf_audio_card_new(pa_bluetooth_backend *backend, 
const char *path) {
 struct hf_audio_card *card = pa_xnew0(struct hf_audio_card, 1);
 
@@ -189,12 +201,18 @@ static struct hf_audio_card 
*hf_audio_card_new(pa_bluetooth_backend *backend, co
 card->fd = -1;
 card->acquire = card_acquire;
 
+card->device_unlink_slot = 
pa_hook_connect(pa_bluetooth_discovery_hook(backend->discovery, 
PA_BLUETOOTH_HOOK_DEVICE_UNLINK),
+   PA_HOOK_NORMAL, (pa_hook_cb_t) 
device_unlink_cb, card);
+
 return card;
 }
 
 static void hf_audio_card_free(struct hf_audio_card *card) {
 pa_assert(card);
 
+if (card->device_unlink_slot)
+pa_hook_slot_free(card->device_unlink_slot);
+
 if (card->transport)
 pa_bluetooth_transport_free(card->transport);
 
diff --git a/src/modules/bluetooth/bluez5-util.c 
b/src/modules/bluetooth/bluez5-util.c
index 3c5a000c0..d95c9c117 100644
--- a/src/modules/bluetooth/bluez5-util.c
+++ b/src/modules/bluetooth/bluez5-util.c
@@ -562,6 +562,8 @@ static void device_free(pa_bluetooth_device *d) {
 
 device_stop_waiting_for_profiles(d);
 
+pa_hook_fire(>discovery->hooks[PA_BLUETOOTH_HOOK_DEVICE_UNLINK], d);
+
 for (i = 0; i < PA_BLUETOOTH_PROFILE_COUNT; i++) {
 pa_bluetooth_transport *t;
 
diff --git a/src/modules/bluetooth/bluez5-util.h 
b/src/modules/bluetooth/bluez5-util.h
index 82739bffd..1e9caecb4 100644
--- a/src/modules/bluetooth/bluez5-util.h
+++ b/src/modules/bluetooth/bluez5-util.h
@@ -50,6 +50,7 @@ typedef enum pa_bluetooth_hook {
 PA_BLUETOOTH_HOOK_TRANSPORT_STATE_CHANGED,/* Call data: 
pa_bluetooth_transport */
 PA_BLUETOOTH_HOOK_TRANSPORT_MICROPHONE_GAIN_CHANGED,  /* Call data: 
pa_bluetooth_transport */
 PA_BLUETOOTH_HOOK_TRANSPORT_SPEAKER_GAIN_CHANGED, /* Call data: 
pa_bluetooth_transport */
+PA_BLUETOOTH_HOOK_DEVICE_UNLINK,  /* Call data: 
pa_bluetooth_device */
 PA_BLUETOOTH_HOOK_MAX
 } pa_bluetooth_hook_t;
 
-- 
2.18.0

___
pulseaudio-discuss mailing list
pulseaudio-discuss@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/pulseaudio-discuss