[glib/1896-use-after-free-when-calling-g_dbus_connection_flush_sync-in-a-dedicated-thread: 9/9] Fix use-after-free when calling g_dbus_connection_flush_sync()
- From: Milan Crha <mcrha src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [glib/1896-use-after-free-when-calling-g_dbus_connection_flush_sync-in-a-dedicated-thread: 9/9] Fix use-after-free when calling g_dbus_connection_flush_sync()
- Date: Thu, 10 Oct 2019 12:15:36 +0000 (UTC)
commit 89b44e5a7547207cd1627acdf8c9b20416add000
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 | 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);
[
Date Prev][
Date Next] [
Thread Prev][
Thread Next]
[
Thread Index]
[
Date Index]
[
Author Index]