From b1a7149819c1d7e71b136840a6d9456ab7b1797c42116106dc4b8b4e360a86c2 Mon Sep 17 00:00:00 2001 From: Luciano Santos Date: Mon, 2 Oct 2023 02:55:19 +0000 Subject: [PATCH] Accepting request 1114027 from home:scabrero:branches:GNOME:Factory - Fix NetworkManager crashing repeatedly with glib 2.78.0; (bsc#1215709); Add patch 0005-gthreadedresolver-Fix-race.patch OBS-URL: https://build.opensuse.org/request/show/1114027 OBS-URL: https://build.opensuse.org/package/show/GNOME:Factory/glib2?expand=0&rev=523 --- 0005-gthreadedresolver-Fix-race.patch | 132 ++++++++++++++++++++++++++ glib2.changes | 6 ++ glib2.spec | 3 + 3 files changed, 141 insertions(+) create mode 100644 0005-gthreadedresolver-Fix-race.patch diff --git a/0005-gthreadedresolver-Fix-race.patch b/0005-gthreadedresolver-Fix-race.patch new file mode 100644 index 0000000..d830880 --- /dev/null +++ b/0005-gthreadedresolver-Fix-race.patch @@ -0,0 +1,132 @@ +From 82c764ce2e42f0d1032627dabcbd742d5f2bd8fa Mon Sep 17 00:00:00 2001 +From: Philip Withnall +Date: Mon, 11 Sep 2023 16:02:15 +0100 +Subject: [PATCH] gthreadedresolver: Fix race between source callbacks and + finalize +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +I had thought that because `g_source_destroy()` was called for the two +sources (cancel and timeout) in the `GTask` finalize function for a +threaded resolver operation, that it would be fine to use a plain +pointer in the source callbacks to point to the `GTask`. + +That turns out to not be true: because the source callbacks are executed +in the GLib worker thread, and the `GTask` can be finalized in another +thread, it’s possible for a source callback (e.g. `cancelled_cb()`) to +be scheduled in the worker thread, then for the `GTask` to be finalized, +and then the source callback to continue execution and find itself +doing a use-after-free. + +Fix that by using a weak ref to the `GTask` in the source callbacks, +rather than a plain pointer. + +Signed-off-by: Philip Withnall + +Fixes: #3105 +--- + gio/gthreadedresolver.c | 43 +++++++++++++++++++++++++++++++++++------ + 1 file changed, 37 insertions(+), 6 deletions(-) + +diff --git a/gio/gthreadedresolver.c b/gio/gthreadedresolver.c +index 2d94531bf..c7a567549 100644 +--- a/gio/gthreadedresolver.c ++++ b/gio/gthreadedresolver.c +@@ -1422,10 +1422,17 @@ lookup_records_finish (GResolver *resolver, + static gboolean + timeout_cb (gpointer user_data) + { +- GTask *task = G_TASK (user_data); +- LookupData *data = g_task_get_task_data (task); ++ GWeakRef *weak_task = user_data; ++ GTask *task = NULL; /* (owned) */ ++ LookupData *data; + gboolean should_return; + ++ task = g_weak_ref_get (weak_task); ++ if (task == NULL) ++ return G_SOURCE_REMOVE; ++ ++ data = g_task_get_task_data (task); ++ + g_mutex_lock (&data->lock); + + should_return = g_atomic_int_compare_and_exchange (&data->will_return, NOT_YET, TIMED_OUT); +@@ -1443,6 +1450,8 @@ timeout_cb (gpointer user_data) + g_cond_broadcast (&data->cond); + g_mutex_unlock (&data->lock); + ++ g_object_unref (task); ++ + return G_SOURCE_REMOVE; + } + +@@ -1452,10 +1461,17 @@ static gboolean + cancelled_cb (GCancellable *cancellable, + gpointer user_data) + { +- GTask *task = G_TASK (user_data); +- LookupData *data = g_task_get_task_data (task); ++ GWeakRef *weak_task = user_data; ++ GTask *task = NULL; /* (owned) */ ++ LookupData *data; + gboolean should_return; + ++ task = g_weak_ref_get (weak_task); ++ if (task == NULL) ++ return G_SOURCE_REMOVE; ++ ++ data = g_task_get_task_data (task); ++ + g_mutex_lock (&data->lock); + + g_assert (g_cancellable_is_cancelled (cancellable)); +@@ -1473,9 +1489,18 @@ cancelled_cb (GCancellable *cancellable, + g_cond_broadcast (&data->cond); + g_mutex_unlock (&data->lock); + ++ g_object_unref (task); ++ + return G_SOURCE_REMOVE; + } + ++static void ++weak_ref_clear_and_free (GWeakRef *weak_ref) ++{ ++ g_weak_ref_clear (weak_ref); ++ g_free (weak_ref); ++} ++ + static void + run_task_in_thread_pool_async (GThreadedResolver *self, + GTask *task) +@@ -1490,17 +1515,23 @@ run_task_in_thread_pool_async (GThreadedResolver *self, + + if (timeout_ms != 0) + { ++ GWeakRef *weak_task = g_new0 (GWeakRef, 1); ++ g_weak_ref_set (weak_task, task); ++ + data->timeout_source = g_timeout_source_new (timeout_ms); + g_source_set_static_name (data->timeout_source, "[gio] threaded resolver timeout"); +- g_source_set_callback (data->timeout_source, G_SOURCE_FUNC (timeout_cb), task, NULL); ++ g_source_set_callback (data->timeout_source, G_SOURCE_FUNC (timeout_cb), g_steal_pointer (&weak_task), (GDestroyNotify) weak_ref_clear_and_free); + g_source_attach (data->timeout_source, GLIB_PRIVATE_CALL (g_get_worker_context) ()); + } + + if (cancellable != NULL) + { ++ GWeakRef *weak_task = g_new0 (GWeakRef, 1); ++ g_weak_ref_set (weak_task, task); ++ + data->cancellable_source = g_cancellable_source_new (cancellable); + g_source_set_static_name (data->cancellable_source, "[gio] threaded resolver cancellable"); +- g_source_set_callback (data->cancellable_source, G_SOURCE_FUNC (cancelled_cb), task, NULL); ++ g_source_set_callback (data->cancellable_source, G_SOURCE_FUNC (cancelled_cb), g_steal_pointer (&weak_task), (GDestroyNotify) weak_ref_clear_and_free); + g_source_attach (data->cancellable_source, GLIB_PRIVATE_CALL (g_get_worker_context) ()); + } + +-- +2.42.0 + diff --git a/glib2.changes b/glib2.changes index 32545d6..6f6b498 100644 --- a/glib2.changes +++ b/glib2.changes @@ -1,3 +1,9 @@ +------------------------------------------------------------------- +Thu Sep 28 07:40:17 UTC 2023 - Samuel Cabrero + +- Fix NetworkManager crashing repeatedly with glib 2.78.0; + (bsc#1215709); Add patch 0005-gthreadedresolver-Fix-race.patch + ------------------------------------------------------------------- Fri Sep 8 19:58:59 UTC 2023 - Bjørn Lie diff --git a/glib2.spec b/glib2.spec index 81c59f1..de3582e 100644 --- a/glib2.spec +++ b/glib2.spec @@ -56,6 +56,8 @@ Patch1: glib2-fate300461-gettext-gkeyfile-suse.patch Patch2: glib2-suppress-schema-deprecated-path-warning.patch # PATCH-FIX-OPENSUSE glib2-gdbus-codegen-version.patch olaf@aepfle.de -- Remove version string from files generated by gdbus-codegen Patch4: glib2-gdbus-codegen-version.patch +# PATCH-FIX-OPENSUSE 0005-gthreadedresolver-Fix-race.patch bsc#1215709 -- Backport patch to fix race between source callbacks and finalize causing NM to crash repeatedly +Patch5: 0005-gthreadedresolver-Fix-race.patch BuildRequires: docbook-xsl-stylesheets BuildRequires: fdupes @@ -258,6 +260,7 @@ the functionality of the installed glib2 package. %patch1 -p1 %patch2 -p1 %patch4 -p1 +%patch5 -p1 cp -a %{SOURCE1} %{SOURCE2} %{SOURCE5} . cp -a %{SOURCE4} gnome_defaults.conf