[Spice-commits] src/channel-main.c

[Jonathon Jongsma]

 src/channel-main.c |   11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

New commits:
commit 401275b10abcf6c9fbbc426b40629eac2202044f
Author: Jonathon Jongsma <jjongsma at redhat.com>
Date:   Thu Nov 5 12:17:24 2015 -0600

    file xfer: Fix segfault when rebooting
    
    Recent changes to file transfer introduced a regression where the client
    would crash when rebooting a guest after performing a file transfer.
    This was caused because the SpiceFileTransferTask is freed when it is
    completed, but is not removed from the MainChannel's hash table. When we
    reboot the guest and lose our vdagent connection, we iterate through the
    list of tasks in the hash table and complete them. But since we did not
    remove the already-completed tasks from this hash table, this hash table
    contains already-freed memory.
    
    To fix the issue, take an extra ref for the async operations (so that
    completing the task won't free an object that is stored in the hash table). In
    addition, connect to the task's "finished" signal and remove it from the hash
    table when it becomes finished.
    
    Bug reported via email by Jay.han <ezzzehxx at gmail.com>. Valgrind report
    below:
    
    ==6926== Invalid read of size 8
    ==6926==    at 0x508177B: spice_file_transfer_task_completed (channel-main.c:2941)
    ==6926==    by 0x50846DC: set_agent_connected (channel-main.c:462)
    ==6926==    by 0x5073A43: spice_channel_recv_msg (spice-channel.c:1892)
    ==6926==    by 0x5073BE3: spice_channel_iterate_read (spice-channel.c:2132)
    ==6926==    by 0x5075D25: spice_channel_coroutine (spice-channel.c:2170)
    ==6926==    by 0x50A6EFE: coroutine_trampoline (coroutine_ucontext.c:63)
    ==6926==    by 0x50A6CC8: continuation_trampoline (continuation.c:55)
    ==6926==    by 0x65C2B5F: ??? (in /lib/x86_64-linux-gnu/libc-2.19.so)
    ==6926==    by 0x151331C7: ???
    ==6926==  Address 0x29971fd8 is 168 bytes inside a block of size 184 free'd
    ==6926==    at 0x4C2BDEC: free (in /usr/lib/valgrind/vgpreload_memcheck-amd64-linux.so)
    ==6926==    by 0x5E33142: g_type_free_instance (in /usr/lib/x86_64-linux-gnu/libgobject-2.0.so.0.4000.0)
    ==6926==    by 0x50815DA: file_xfer_close_cb (channel-main.c:1826)
    ==6926==    by 0x5AEBD5C: ??? (in /usr/lib/x86_64-linux-gnu/libgio-2.0.so.0.4000.0)
    ==6926==    by 0x5B0F41A: ??? (in /usr/lib/x86_64-linux-gnu/libgio-2.0.so.0.4000.0)
    ==6926==    by 0x5B0F438: ??? (in /usr/lib/x86_64-linux-gnu/libgio-2.0.so.0.4000.0)
    ==6926==    by 0x609BCE4: g_main_context_dispatch (in /lib/x86_64-linux-gnu/libglib-2.0.so.0.4000.0)
    ==6926==    by 0x609C047: ??? (in /lib/x86_64-linux-gnu/libglib-2.0.so.0.4000.0)
    ==6926==    by 0x609C309: g_main_loop_run (in /lib/x86_64-linux-gnu/libglib-2.0.so.0.4000.0)
    ==6926==    by 0x4058AB: main (spicy.c:1858)

diff --git a/src/channel-main.c b/src/channel-main.c
index 0eb40b9..8138fd5 100644
--- a/src/channel-main.c
+++ b/src/channel-main.c
@@ -3041,6 +3041,14 @@ static SpiceFileTransferTask *spice_file_transfer_task_new(SpiceMainChannel *cha
                                                            GFile *file,
                                                            GCancellable *cancellable);
 
+static void task_finished(SpiceFileTransferTask *task,
+                          GError *error,
+                          gpointer data)
+{
+    SpiceMainChannel *channel = SPICE_MAIN_CHANNEL(data);
+    g_hash_table_remove(channel->priv->file_xfer_tasks, GUINT_TO_POINTER(task->priv->id));
+}
+
 static void file_xfer_send_start_msg_async(SpiceMainChannel *channel,
                                            GFile **files,
                                            GFileCopyFlags flags,
@@ -3074,13 +3082,14 @@ static void file_xfer_send_start_msg_async(SpiceMainChannel *channel,
         g_hash_table_insert(c->file_xfer_tasks,
                             GUINT_TO_POINTER(task->priv->id),
                             task);
+        g_signal_connect(task, "finished", G_CALLBACK(task_finished), channel);
         g_signal_emit(channel, signals[SPICE_MAIN_NEW_FILE_TRANSFER], 0, task);
 
         g_file_read_async(files[i],
                           G_PRIORITY_DEFAULT,
                           cancellable,
                           file_xfer_read_async_cb,
-                          task);
+                          g_object_ref(task));
         task->priv->pending = TRUE;
 
         /* if we created a per-task cancellable above, free it */