| Message ID | 20200821073714.19626-1-sonnysasaka@chromium.org (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | [BlueZ] adapter: Fix crash in discovery_disconnect | expand |
Hi Sonny, On Fri, Aug 21, 2020 at 12:42 AM Sonny Sasaka <sonnysasaka@chromium.org> wrote: > > discovery_disconnect crashed because the adapter pointer has been freed > before. This patch makes sure that discovery list is cleaned up before > adapter pointer is freed. > > Reviewed-by: Miao-chen Chou <mcchou@chromium.org> > > --- > src/adapter.c | 19 ++++++++++++++----- > 1 file changed, 14 insertions(+), 5 deletions(-) > > diff --git a/src/adapter.c b/src/adapter.c > index 5e896a9f0..c0b02bf3f 100644 > --- a/src/adapter.c > +++ b/src/adapter.c > @@ -5316,12 +5316,25 @@ static void free_service_auth(gpointer data, gpointer user_data) > g_free(auth); > } > > +static void remove_discovery_list(struct btd_adapter *adapter) > +{ > + g_slist_free_full(adapter->set_filter_list, discovery_free); > + adapter->set_filter_list = NULL; > + > + g_slist_free_full(adapter->discovery_list, discovery_free); > + adapter->discovery_list = NULL; > +} > + > static void adapter_free(gpointer user_data) > { > struct btd_adapter *adapter = user_data; > > DBG("%p", adapter); > > + // Make sure the adapter's discovery list is cleaned up before freeing > + // the adapter. Please use C style comments. > + remove_discovery_list(adapter); > + > if (adapter->pairable_timeout_id > 0) { > g_source_remove(adapter->pairable_timeout_id); > adapter->pairable_timeout_id = 0; > @@ -6846,11 +6859,7 @@ static void adapter_stop(struct btd_adapter *adapter) > > cancel_passive_scanning(adapter); > > - g_slist_free_full(adapter->set_filter_list, discovery_free); > - adapter->set_filter_list = NULL; > - > - g_slist_free_full(adapter->discovery_list, discovery_free); > - adapter->discovery_list = NULL; > + remove_discovery_list(adapter); > > discovery_cleanup(adapter, 0); > > -- > 2.26.2 Otherwise it looks good.
Hi Luiz, Thanks for the feedback, I have sent a v2 of this patch. On Fri, Aug 21, 2020 at 10:38 AM Luiz Augusto von Dentz <luiz.dentz@gmail.com> wrote: > > Hi Sonny, > > On Fri, Aug 21, 2020 at 12:42 AM Sonny Sasaka <sonnysasaka@chromium.org> wrote: > > > > discovery_disconnect crashed because the adapter pointer has been freed > > before. This patch makes sure that discovery list is cleaned up before > > adapter pointer is freed. > > > > Reviewed-by: Miao-chen Chou <mcchou@chromium.org> > > > > --- > > src/adapter.c | 19 ++++++++++++++----- > > 1 file changed, 14 insertions(+), 5 deletions(-) > > > > diff --git a/src/adapter.c b/src/adapter.c > > index 5e896a9f0..c0b02bf3f 100644 > > --- a/src/adapter.c > > +++ b/src/adapter.c > > @@ -5316,12 +5316,25 @@ static void free_service_auth(gpointer data, gpointer user_data) > > g_free(auth); > > } > > > > +static void remove_discovery_list(struct btd_adapter *adapter) > > +{ > > + g_slist_free_full(adapter->set_filter_list, discovery_free); > > + adapter->set_filter_list = NULL; > > + > > + g_slist_free_full(adapter->discovery_list, discovery_free); > > + adapter->discovery_list = NULL; > > +} > > + > > static void adapter_free(gpointer user_data) > > { > > struct btd_adapter *adapter = user_data; > > > > DBG("%p", adapter); > > > > + // Make sure the adapter's discovery list is cleaned up before freeing > > + // the adapter. > > Please use C style comments. > > > + remove_discovery_list(adapter); > > + > > if (adapter->pairable_timeout_id > 0) { > > g_source_remove(adapter->pairable_timeout_id); > > adapter->pairable_timeout_id = 0; > > @@ -6846,11 +6859,7 @@ static void adapter_stop(struct btd_adapter *adapter) > > > > cancel_passive_scanning(adapter); > > > > - g_slist_free_full(adapter->set_filter_list, discovery_free); > > - adapter->set_filter_list = NULL; > > - > > - g_slist_free_full(adapter->discovery_list, discovery_free); > > - adapter->discovery_list = NULL; > > + remove_discovery_list(adapter); > > > > discovery_cleanup(adapter, 0); > > > > -- > > 2.26.2 > > Otherwise it looks good. > > -- > Luiz Augusto von Dentz
diff --git a/src/adapter.c b/src/adapter.c index 5e896a9f0..c0b02bf3f 100644 --- a/src/adapter.c +++ b/src/adapter.c @@ -5316,12 +5316,25 @@ static void free_service_auth(gpointer data, gpointer user_data) g_free(auth); } +static void remove_discovery_list(struct btd_adapter *adapter) +{ + g_slist_free_full(adapter->set_filter_list, discovery_free); + adapter->set_filter_list = NULL; + + g_slist_free_full(adapter->discovery_list, discovery_free); + adapter->discovery_list = NULL; +} + static void adapter_free(gpointer user_data) { struct btd_adapter *adapter = user_data; DBG("%p", adapter); + // Make sure the adapter's discovery list is cleaned up before freeing + // the adapter. + remove_discovery_list(adapter); + if (adapter->pairable_timeout_id > 0) { g_source_remove(adapter->pairable_timeout_id); adapter->pairable_timeout_id = 0; @@ -6846,11 +6859,7 @@ static void adapter_stop(struct btd_adapter *adapter) cancel_passive_scanning(adapter); - g_slist_free_full(adapter->set_filter_list, discovery_free); - adapter->set_filter_list = NULL; - - g_slist_free_full(adapter->discovery_list, discovery_free); - adapter->discovery_list = NULL; + remove_discovery_list(adapter); discovery_cleanup(adapter, 0);
discovery_disconnect crashed because the adapter pointer has been freed before. This patch makes sure that discovery list is cleaned up before adapter pointer is freed. Reviewed-by: Miao-chen Chou <mcchou@chromium.org> --- src/adapter.c | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-)