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@xxxxxxxxx>. 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)
---
src/channel-main.c | 13 ++++++++++++-
1 file changed, 12 insertions(+), 1 deletion(-)
diff --git a/src/channel-main.c b/src/channel-main.c
index 0eb40b9..5abb32d 100644
--- a/src/channel-main.c
+++ b/src/channel-main.c
@@ -3041,6 +3041,16 @@ 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_debug("%s: %i", G_STRFUNC, task->priv->id);
+
+ 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 +3084,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 */
--
2.4.3
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel