Digi
/
meta-digi
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

glib-2.0: backport 2.65 patch to prevent NetworkManager segmentation faults 

NetworkManager's main library went through a major overhaul in v1.22, changing
the way it interacts with glib among other things. When using a NetworkManager
version equal or newer than v1.22 along with a glib version between 2.63.3 and
2.65, a race condition can happen, randomly causing segmentation faults.

Since Yocto 3.2 uses NetworkManager 1.22.14 and glib 2.64.5, the race condition
is reproducible, but it can be fixed with the patch introduced in this commit.
The patch in question is commit e4a690f5dd959e74b2d6054826f61509892c8aa7 in the
glib git repo.

https://onedigi.atlassian.net/browse/DEL-7523

Signed-off-by: Gabriel Valcazar <gabriel.valcazar@digi.com>
This commit is contained in:
Gabriel Valcazar 2021-08-10 09:05:35 +02:00
parent 6462549f04
commit feec2aa4f7
2 changed files with 219 additions and 0 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

216
meta-digi-dey/recipes-core/glib-2.0/glib-2.0/0001-gcancellable-Fix-minor-race-between-GCancellable-and.patch Normal file
View File

@ -0,0 +1,216 @@
From: Philip Withnall <withnall@endlessm.com>
Date: Fri, 21 Feb 2020 14:44:44 +0000
Subject: [PATCH] gcancellable: Fix minor race between GCancellable and
GCancellableSource
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
There’s a minor race condition between cancellation of a `GCancellable`,
and disposal/finalisation of a `GCancellableSource` in another thread.
Thread A Thread B
g_cancellable_cancel(C)
→cancellable_source_cancelled(C, S)
g_source_unref(S)
cancellable_source_dispose(S)
→→g_source_ref(S)
→→# S is invalid at this point; crash
Thankfully, the `GCancellable` sets `cancelled_running` while it’s
emitting the `cancelled` signal, so if `cancellable_source_dispose()` is
called while that’s high, we know that the thread which is doing the
cancellation has already started (or is committed to starting) calling
`cancellable_source_cancelled()`.
Fix the race by resurrecting the `GCancellableSource` in
`cancellable_source_dispose()`, and signalling this using
`GCancellableSource.resurrected_during_cancellation`. Check for that
flag in `cancellable_source_cancelled()` and ignore cancellation if it’s
set.
The modifications to `resurrected_during_cancellation` and the
cancellable source’s refcount have to be done with `cancellable_mutex`
held so that they are seen atomically by each thread. This should not
affect performance too much, as it only happens during cancellation or
disposal of a `GCancellableSource`.
Signed-off-by: Philip Withnall <withnall@endlessm.com>
Fixes: #1841
---
gio/gcancellable.c | 43 +++++++++++++++++++++++
gio/tests/cancellable.c | 77 +++++++++++++++++++++++++++++++++++++++++
2 files changed, 120 insertions(+)
diff --git a/gio/gcancellable.c b/gio/gcancellable.c
index d9e58b8e8..e687cca23 100644
--- a/gio/gcancellable.c
+++ b/gio/gcancellable.c
@@ -643,6 +643,8 @@ typedef struct {
GCancellable *cancellable;
gulong cancelled_handler;
+ /* Protected by cancellable_mutex: */
+ gboolean resurrected_during_cancellation;
} GCancellableSource;
/*
@@ -661,8 +663,24 @@ cancellable_source_cancelled (GCancellable *cancellable,
gpointer user_data)
{
GSource *source = user_data;
+ GCancellableSource *cancellable_source = (GCancellableSource *) source;
+
+ g_mutex_lock (&cancellable_mutex);
+
+ /* Drop the reference added in cancellable_source_dispose(); see the comment there.
+ * The reference must be dropped after unlocking @cancellable_mutex since
+ * it could be the final reference, and the dispose function takes
+ * @cancellable_mutex. */
+ if (cancellable_source->resurrected_during_cancellation)
+ {
+ cancellable_source->resurrected_during_cancellation = FALSE;
+ g_mutex_unlock (&cancellable_mutex);
+ g_source_unref (source);
+ return;
+ }
g_source_ref (source);
+ g_mutex_unlock (&cancellable_mutex);
g_source_set_ready_time (source, 0);
g_source_unref (source);
}
@@ -684,12 +702,37 @@ cancellable_source_dispose (GSource *source)
{
GCancellableSource *cancellable_source = (GCancellableSource *)source;
+ g_mutex_lock (&cancellable_mutex);
+
if (cancellable_source->cancellable)
{
+ if (cancellable_source->cancellable->priv->cancelled_running)
+ {
+ /* There can be a race here: if thread A has called
+ * g_cancellable_cancel() and has got as far as committing to call
+ * cancellable_source_cancelled(), then thread B drops the final
+ * ref on the GCancellableSource before g_source_ref() is called in
+ * cancellable_source_cancelled(), then cancellable_source_dispose()
+ * will run through and the GCancellableSource will be finalised
+ * before cancellable_source_cancelled() gets to g_source_ref(). It
+ * will then be left in a state where it’s committed to using a
+ * dangling GCancellableSource pointer.
+ *
+ * Eliminate that race by resurrecting the #GSource temporarily, and
+ * then dropping that reference in cancellable_source_cancelled(),
+ * which should be guaranteed to fire because we’re inside a
+ * @cancelled_running block.
+ */
+ g_source_ref (source);
+ cancellable_source->resurrected_during_cancellation = TRUE;
+ }
+
g_clear_signal_handler (&cancellable_source->cancelled_handler,
cancellable_source->cancellable);
g_clear_object (&cancellable_source->cancellable);
}
+
+ g_mutex_unlock (&cancellable_mutex);
}
static gboolean
diff --git a/gio/tests/cancellable.c b/gio/tests/cancellable.c
index cd349a8f3..492704d18 100644
--- a/gio/tests/cancellable.c
+++ b/gio/tests/cancellable.c
@@ -228,6 +228,82 @@ test_cancel_null (void)
g_cancellable_cancel (NULL);
}
+typedef struct
+{
+ GCond cond;
+ GMutex mutex;
+ GSource *cancellable_source; /* (owned) */
+} ThreadedDisposeData;
+
+static gboolean
+cancelled_cb (GCancellable *cancellable,
+ gpointer user_data)
+{
+ /* Nothing needs to be done here. */
+ return G_SOURCE_CONTINUE;
+}
+
+static gpointer
+threaded_dispose_thread_cb (gpointer user_data)
+{
+ ThreadedDisposeData *data = user_data;
+
+ /* Synchronise with the main thread before trying to reproduce the race. */
+ g_mutex_lock (&data->mutex);
+ g_cond_broadcast (&data->cond);
+ g_mutex_unlock (&data->mutex);
+
+ /* Race with cancellation of the cancellable. */
+ g_source_unref (data->cancellable_source);
+
+ return NULL;
+}
+
+static void
+test_cancellable_source_threaded_dispose (void)
+{
+ guint i;
+
+ g_test_summary ("Test a thread race between disposing of a GCancellableSource "
+ "(in one thread) and cancelling the GCancellable it refers "
+ "to (in another thread)");
+ g_test_bug ("https://gitlab.gnome.org/GNOME/glib/issues/1841");
+
+ for (i = 0; i < 100000; i++)
+ {
+ GCancellable *cancellable = NULL;
+ GSource *cancellable_source = NULL;
+ ThreadedDisposeData data;
+ GThread *thread = NULL;
+
+ /* Create a cancellable and a cancellable source for it. For this test,
+ * there’s no need to attach the source to a #GMainContext. */
+ cancellable = g_cancellable_new ();
+ cancellable_source = g_cancellable_source_new (cancellable);
+ g_source_set_callback (cancellable_source, G_SOURCE_FUNC (cancelled_cb), NULL, NULL);
+
+ /* Create a new thread and wait until it’s ready to execute before
+ * cancelling our cancellable. */
+ g_cond_init (&data.cond);
+ g_mutex_init (&data.mutex);
+ data.cancellable_source = g_steal_pointer (&cancellable_source);
+
+ g_mutex_lock (&data.mutex);
+ thread = g_thread_new ("/cancellable-source/threaded-dispose",
+ threaded_dispose_thread_cb, &data);
+ g_cond_wait (&data.cond, &data.mutex);
+ g_mutex_unlock (&data.mutex);
+
+ /* Race with disposal of the cancellable source. */
+ g_cancellable_cancel (cancellable);
+
+ g_thread_join (g_steal_pointer (&thread));
+ g_mutex_clear (&data.mutex);
+ g_cond_clear (&data.cond);
+ g_object_unref (cancellable);
+ }
+}
+
int
main (int argc, char *argv[])
{
@@ -235,6 +311,7 @@ main (int argc, char *argv[])
g_test_add_func ("/cancellable/multiple-concurrent", test_cancel_multiple_concurrent);
g_test_add_func ("/cancellable/null", test_cancel_null);
+ g_test_add_func ("/cancellable-source/threaded-dispose", test_cancellable_source_threaded_dispose);
return g_test_run ();
}

3
meta-digi-dey/recipes-core/glib-2.0/glib-2.0_2.64.5.bbappend Normal file
View File

@ -0,0 +1,3 @@
FILESEXTRAPATHS_prepend := "${THISDIR}/${BPN}:"
SRC_URI += " file://0001-gcancellable-Fix-minor-race-between-GCancellable-and.patch"