[glib: 1/2] Fix use-after-free triggered by gnome-session-binary



commit 052ad6098ded95418704a27f487adf7bf97e14b1
Author: Stefan Sperling <stsp stsp name>
Date:   Sat Mar 23 12:07:47 2019 +0100

    Fix use-after-free triggered by gnome-session-binary
    
    ostream_flush_cb() was calling flush_data_list_complete() with a single
    element list with an item that had already been freed. This was observed
    on OpenBSD where memory is overwritten with 0xdf during free():
    
        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

 gio/gdbusprivate.c | 30 ++++++++++++++++++++----------
 1 file changed, 20 insertions(+), 10 deletions(-)
---
diff --git a/gio/gdbusprivate.c b/gio/gdbusprivate.c
index 4d7ca6f89..dee1f2719 100644
--- a/gio/gdbusprivate.c
+++ b/gio/gdbusprivate.c
@@ -1194,13 +1194,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);
@@ -1209,6 +1202,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);
 
@@ -1377,6 +1376,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)
@@ -1792,10 +1795,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)


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]