[glib: 1/2] Fix use-after-free triggered by gnome-session-binary
- From: Philip Withnall <pwithnall src gnome org>
- To: commits-list gnome org
- Cc:
- Subject: [glib: 1/2] Fix use-after-free triggered by gnome-session-binary
- Date: Wed, 3 Apr 2019 12:01:22 +0000 (UTC)
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]