From 822f8bae9e8b2bea7e43f9e010d9240b0f473e2c Mon Sep 17 00:00:00 2001 From: Milan Crha Date: Thu, 10 Oct 2019 14:55:20 +0000 Subject: [PATCH] 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 | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/gio/gdbusprivate.c b/gio/gdbusprivate.c index 0421ca56c..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,6 +1789,7 @@ _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); @@ -1796,14 +1799,10 @@ _g_dbus_worker_flush_sync (GDBusWorker *worker, if (data != NULL) { /* Wait for flush operations to finish. */ - g_mutex_lock (&worker->write_lock); - 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);