luc14n0/glib
1
0
Fork 0
mirror of https://gitlab.gnome.org/GNOME/glib.git synced 2025-01-12 15:36:17 +01:00
Code Issues Packages Projects Releases Wiki Activity

Fix use-after-free triggered by gnome-session-binary

Browse Source 
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
This commit is contained in:
Stefan Sperling 2019-03-23 12:07:47 +01:00
parent ecad5cd522
commit 052ad6098d
1 changed files with 20 additions and 10 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

30
gio/gdbusprivate.c
View File

@ -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)