Re: devel/glib2: fix use-after-free
OK
Re: devel/glib2: fix use-after-free
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
--- Makefile23 Mar 2019 08:51:15 - 1.297
+++ Makefile25 Mar 2019 08:43:08 -
@@ -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_c23 Mar 2019 08:51:15 - 1.1
+++ patches/patch-gio_gdbusprivate_c25 Mar 2019 08:43:28 -
@@ -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 (>worker->write_lock);
+@@ -1205,6 +1198,12 @@ ostream_flush_cb (GObject *source_object,
+ data->worker->output_pending = PENDING_NONE;
+ g_mutex_unlock (>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 (>write_lock);
+
+ while (pending_close_attempts != NULL)
+@@ -1788,10 +1791,17 @@ _g_dbus_worker_flush_sync (GDBusWorker*worker,
if (data != NULL)
{
devel/glib2: fix use-after-free
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
--- Makefile19 Feb 2019 14:53:17 - 1.296
+++ Makefile22 Mar 2019 17:39:57 -
@@ -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 -
+++ patches/patch-gio_gdbusprivate_c22 Mar 2019 19:11:48 -
@@ -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 0x0e3c0104577a in flush_data_list_complete (flushers=0xe3bd8300340,
+error=0x0) at ../glib-2.58.3/gio/gdbusprivate.c:1156
+1156 g_mutex_lock (>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 (>cond, >mutex);
+- g_mutex_unlock (>mutex);
++ /* Wait for flush operations to finish. */
++ g_mutex_lock (>write_lock);
++ while (worker->write_num_messages_flushed < data->number_to_wait_for)
++{
++ g_mutex_unlock (>write_lock);
++ g_cond_wait (>cond, >mutex);
++ g_mutex_lock (>write_lock);
++}
++ g_mutex_unlock (>write_lock);
+
+- /* note:the element is removed from worker->write_pending_flushes in
flush_cb() above */
++ g_mutex_unlock (>mutex);
+ g_cond_clear (>cond);
+ g_mutex_clear (>mutex);
+ if (data->error != NULL)
