[glib/1896-use-after-free-when-calling-g_dbus_connection_flush_sync-in-a-dedicated-thread] Fix use-after-free when calling g_dbus_connection_flush_sync()



commit fff04d79d22d047a6698c48fe0364cf1660bfea3
Author: Milan Crha <mcrha redhat com>
Date:   Thu Oct 10 11:18:11 2019 +0200

    Fix use-after-free when calling g_dbus_connection_flush_sync()
    
    When the _g_dbus_worker_flush_sync() schedules the 'data' and releases
    the worker->write_lock, it is possible for the GDBus worker thread thread
    to finish the D-Bus call and acquire the worker->write_lock before
    the _g_dbus_worker_flush_sync() re-acquires it in the if (data != NULL) body.
    When that happens, the ostream_flush_cb() increases the worker->write_num_messages_flushed
    and then releases the worker->write_lock. The write lock is reacquired by
    the _g_dbus_worker_flush_sync(), which sees that the while condition is satisfied,
    thus it doesn't enter the loop body and immediately clears the data members and
    frees the data structure itself. The ostream_flush_cb() is still ongoing, possibly
    inside flush_data_list_complete(), where it accesses the FlushData, which can be
    in any stage of being freed.
    
    Instead, add an explicit boolean flag indicating when the flush is truly finished.
    
    Closes #1896

 gio/gdbusprivate.c | 11 +++++------
 1 file changed, 5 insertions(+), 6 deletions(-)
---
diff --git a/gio/gdbusprivate.c b/gio/gdbusprivate.c
index 6a524c37b..302c30b42 100644
--- a/gio/gdbusprivate.c
+++ b/gio/gdbusprivate.c
@@ -408,6 +408,7 @@ typedef struct
   GMutex  mutex;
   GCond   cond;
   guint64 number_to_wait_for;
+  gboolean finished;
   GError *error;
 } FlushData;
 
@@ -1158,6 +1159,7 @@ flush_data_list_complete (const GList  *flushers,
       f->error = error != NULL ? g_error_copy (error) : NULL;
 
       g_mutex_lock (&f->mutex);
+      f->finished = TRUE;
       g_cond_signal (&f->cond);
       g_mutex_unlock (&f->mutex);
     }
@@ -1787,21 +1789,20 @@ _g_dbus_worker_flush_sync (GDBusWorker    *worker,
       g_mutex_init (&data->mutex);
       g_cond_init (&data->cond);
       data->number_to_wait_for = worker->write_num_messages_written + pending_writes;
+      data->finished = FALSE;
       g_mutex_lock (&data->mutex);
 
       schedule_writing_unlocked (worker, NULL, data, NULL);
     }
+  g_mutex_unlock (&worker->write_lock);
 
   if (data != NULL)
     {
       /* Wait for flush operations to finish. */
-      while (worker->write_num_messages_flushed < data->number_to_wait_for)
+      while (!data->finished)
         {
-          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);
 
       g_mutex_unlock (&data->mutex);
       g_cond_clear (&data->cond);
@@ -1813,8 +1814,6 @@ _g_dbus_worker_flush_sync (GDBusWorker    *worker,
         }
       g_free (data);
     }
-   else
-    g_mutex_unlock (&worker->write_lock);
 
   return ret;
 }


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