devel/glib2: fix use-after-free

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

devel/glib2: fix use-after-free

Stefan Sperling-5
For some time now I have noticed that logging into gnome from gdm
sometimes failed on my systems, even with my user account in the
'staff' group to avoid the known problems with default resource limits.
Gnome failed to start, and often gdm would not restart properly when
this happened, leaving me with just a console screen.

Today I found a core file from gnome-session-binary and with this
I could track the above problem down to a use-after-free in glib2.

Without the patch below I quickly get a gnome-session-bi.core file in
my home directory if I keep logging in and out of gnome. A nice way
of repeating this is to add the lines below to /etc/gdm/custom.conf,
then logging in via SSH and running '/etc/rc.d/gdm restart' repeatedly.

 TimedLoginEnable=True
 TimedLogin=youruser
 TimedLoginDelay=1

With this patch I can restart gnome like this over and over and the
desktop comes back much more reliably. I don't get a core file anymore.

It is still not entirely free of problems, e.g. gnome-shell sometimes
won't come back after restarting gdm, and then the screen stays black:
# pgrep -fl gdm
24802 /usr/local/sbin/gdm
# pgrep -fl gnome-shell
#
But that looks like a separate issue.

ok?

Index: Makefile
===================================================================
RCS file: /cvs/ports/devel/glib2/Makefile,v
retrieving revision 1.296
diff -u -p -r1.296 Makefile
--- Makefile 19 Feb 2019 14:53:17 -0000 1.296
+++ Makefile 22 Mar 2019 17:39:57 -0000
@@ -9,7 +9,7 @@ COMMENT= general-purpose utility librar
 GNOME_PROJECT= glib
 GNOME_VERSION= 2.58.3
 PKGNAME= ${DISTNAME:S/glib/glib2/}
-REVISION= 5
+REVISION= 6
 
 CATEGORIES= devel
 
Index: patches/patch-gio_gdbusprivate_c
===================================================================
RCS file: patches/patch-gio_gdbusprivate_c
diff -N patches/patch-gio_gdbusprivate_c
--- /dev/null 1 Jan 1970 00:00:00 -0000
+++ patches/patch-gio_gdbusprivate_c 22 Mar 2019 19:11:48 -0000
@@ -0,0 +1,43 @@
+$OpenBSD$
+Fix use-after-free triggered by gnome-session-binary when Gnome restarts.
+
+ostream_flush_cb() was calling flush_data_list_complete() with a single
+element list with an item that had already been freed:
+
+#2  0x00000e3c0104577a in flush_data_list_complete (flushers=0xe3bd8300340,
+    error=0x0) at ../glib-2.58.3/gio/gdbusprivate.c:1156
+1156          g_mutex_lock (&f->mutex);
+(gdb) p /x *f
+$74 = {mutex = {p = 0xdfdfdfdfdfdfdfdf, i = {0xdfdfdfdf, 0xdfdfdfdf}}, cond = {
+    p = 0xdfdfdfdfdfdfdfdf, i = {0xdfdfdfdf, 0xdfdfdfdf}},
+  number_to_wait_for = 0xdfdfdfdfdfdfdfdf, error = 0x0}
+
+This happened because the thread freeing the element didn't properly wait
+for the asynchronous flush operation to finish.
+Gnome's developer docs say: "g_cond_wait() must always be used in a loop"
+https://developer.gnome.org/glib/stable/glib-Threads.html#g-cond-wait
+
+Index: gio/gdbusprivate.c
+--- gio/gdbusprivate.c.orig
++++ gio/gdbusprivate.c
+@@ -1788,10 +1788,17 @@ _g_dbus_worker_flush_sync (GDBusWorker    *worker,
+
+   if (data != NULL)
+     {
+-      g_cond_wait (&data->cond, &data->mutex);
+-      g_mutex_unlock (&data->mutex);
++      /* Wait for flush operations to finish. */
++      g_mutex_lock (&worker->write_lock);
++      while (worker->write_num_messages_flushed < data->number_to_wait_for)
++        {
++          g_mutex_unlock (&worker->write_lock);
++          g_cond_wait (&data->cond, &data->mutex);
++          g_mutex_lock (&worker->write_lock);
++        }
++      g_mutex_unlock (&worker->write_lock);
+
+-      /* note:the element is removed from worker->write_pending_flushes in flush_cb() above */
++      g_mutex_unlock (&data->mutex);
+       g_cond_clear (&data->cond);
+       g_mutex_clear (&data->mutex);
+       if (data->error != NULL)

Reply | Threaded
Open this post in threaded view
|

Re: devel/glib2: fix use-after-free

Stefan Sperling-5
On Fri, Mar 22, 2019 at 08:22:05PM +0100, Stefan Sperling wrote:
> For some time now I have noticed that logging into gnome from gdm
> sometimes failed on my systems, even with my user account in the
> 'staff' group to avoid the known problems with default resource limits.
> Gnome failed to start, and often gdm would not restart properly when
> this happened, leaving me with just a console screen.
>
> Today I found a core file from gnome-session-binary and with this
> I could track the above problem down to a use-after-free in glib2.

This patch has been added to the ports tree, and then submitted
upstream where problems were caught by regression tests:
https://gitlab.gnome.org/GNOME/glib/merge_requests/741/

This updates our port to a new version of my patch which passes
upstream's regression tests.

Index: Makefile
===================================================================
RCS file: /cvs/ports/devel/glib2/Makefile,v
retrieving revision 1.297
diff -u -p -r1.297 Makefile
--- Makefile 23 Mar 2019 08:51:15 -0000 1.297
+++ Makefile 25 Mar 2019 08:43:08 -0000
@@ -9,7 +9,7 @@ COMMENT= general-purpose utility librar
 GNOME_PROJECT= glib
 GNOME_VERSION= 2.58.3
 PKGNAME= ${DISTNAME:S/glib/glib2/}
-REVISION= 6
+REVISION= 7
 
 CATEGORIES= devel
 
Index: patches/patch-gio_gdbusprivate_c
===================================================================
RCS file: /cvs/ports/devel/glib2/patches/patch-gio_gdbusprivate_c,v
retrieving revision 1.1
diff -u -p -r1.1 patch-gio_gdbusprivate_c
--- patches/patch-gio_gdbusprivate_c 23 Mar 2019 08:51:15 -0000 1.1
+++ patches/patch-gio_gdbusprivate_c 25 Mar 2019 08:43:28 -0000
@@ -1,5 +1,6 @@
 $OpenBSD: patch-gio_gdbusprivate_c,v 1.1 2019/03/23 08:51:15 stsp Exp $
 Fix use-after-free triggered by gnome-session-binary when Gnome restarts.
+https://gitlab.gnome.org/GNOME/glib/merge_requests/741/
 
 ostream_flush_cb() was calling flush_data_list_complete() with a single
 element list with an item that had already been freed:
@@ -20,7 +21,45 @@ https://developer.gnome.org/glib/stable/
 Index: gio/gdbusprivate.c
 --- gio/gdbusprivate.c.orig
 +++ gio/gdbusprivate.c
-@@ -1788,10 +1788,17 @@ _g_dbus_worker_flush_sync (GDBusWorker    *worker,
+@@ -1190,13 +1190,6 @@ ostream_flush_cb (GObject      *source_object,
+         }
+     }
+
+-  g_assert (data->flushers != NULL);
+-  flush_data_list_complete (data->flushers, error);
+-  g_list_free (data->flushers);
+-
+-  if (error != NULL)
+-    g_error_free (error);
+-
+   /* Make sure we tell folks that we don't have additional
+      flushes pending */
+   g_mutex_lock (&data->worker->write_lock);
+@@ -1205,6 +1198,12 @@ ostream_flush_cb (GObject      *source_object,
+   data->worker->output_pending = PENDING_NONE;
+   g_mutex_unlock (&data->worker->write_lock);
+
++  g_assert (data->flushers != NULL);
++  flush_data_list_complete (data->flushers, error);
++  g_list_free (data->flushers);
++  if (error != NULL)
++    g_error_free (error);
++
+   /* OK, cool, finally kick off the next write */
+   continue_writing (data->worker);
+
+@@ -1373,6 +1372,10 @@ iostream_close_cb (GObject      *source_object,
+   g_assert (worker->output_pending == PENDING_CLOSE);
+   worker->output_pending = PENDING_NONE;
+
++  /* Ensure threads waiting for pending flushes to finish will be unblocked. */
++  worker->write_num_messages_flushed =
++    worker->write_num_messages_written + g_list_length(pending_flush_attempts);
++
+   g_mutex_unlock (&worker->write_lock);
+
+   while (pending_close_attempts != NULL)
+@@ -1788,10 +1791,17 @@ _g_dbus_worker_flush_sync (GDBusWorker    *worker,
 
    if (data != NULL)
      {

Reply | Threaded
Open this post in threaded view
|

Re: devel/glib2: fix use-after-free

Klemens Nanni-2
OK