[virt-tools-list] [PATCH v3] remote-viewer: fix free on dangling pointer

[Victor Toso]

From: Victor Toso <me at victortoso.com>

On remote_viewer_session_connected() we are passing a dup of URI of
connection and freeing it afterwards. Problem is, we don't disconnect
from listening "session-connected" and on an eventual second emission
of this signal, remote-viewer crashes as seen in the backtrace below.

This can happen over switch-host migration message from
SpiceMainChannel.

To fix the issue, use VirtViewerApp URI information instead of passing
a dup char*.

Found it while improving migrate.py from spice/tests (server):
 | Invalid free() / delete / delete[] / realloc()
 |    at 0x4839A0C: free (vg_replace_malloc.c:540)
 |    by 0x56EBD8C: g_free (in /usr/lib64/libglib-2.0.so.0.6000.6)
 |    by 0x11DED0: remote_viewer_session_connected (remote-viewer.c:658)
 |    by 0x564D741: g_closure_invoke (in /usr/lib64/libgobject-2.0.so.0.6000.6)
 |    by 0x56614F3: ??? (in /usr/lib64/libgobject-2.0.so.0.6000.6)
 |    by 0x566A34D: g_signal_emit_valist (in /usr/lib64/libgobject-2.0.so.0.6000.6)
 |    by 0x566AF68: g_signal_emit_by_name (in /usr/lib64/libgobject-2.0.so.0.6000.6)
 |    by 0x135E5D: virt_viewer_session_spice_main_channel_event (virt-viewer-session-spice.c:699)
 |    by 0x564D741: g_closure_invoke (in /usr/lib64/libgobject-2.0.so.0.6000.6)
 |    by 0x56614F3: ??? (in /usr/lib64/libgobject-2.0.so.0.6000.6)
 |    by 0x566A34D: g_signal_emit_valist (in /usr/lib64/libgobject-2.0.so.0.6000.6)
 |    by 0x53149E3: emit_main_context (gio-coroutine.c:198)
 |  Address 0x18f1ecc0 is 0 bytes inside a block of size 23 free'd
 |    at 0x4839A0C: free (vg_replace_malloc.c:540)
 |    by 0x56EBD8C: g_free (in /usr/lib64/libglib-2.0.so.0.6000.6)
 |    by 0x11DED0: remote_viewer_session_connected (remote-viewer.c:658)
 |    by 0x564D741: g_closure_invoke (in /usr/lib64/libgobject-2.0.so.0.6000.6)
 |    by 0x56614F3: ??? (in /usr/lib64/libgobject-2.0.so.0.6000.6)
 |    by 0x566A34D: g_signal_emit_valist (in /usr/lib64/libgobject-2.0.so.0.6000.6)
 |    by 0x566AF68: g_signal_emit_by_name (in /usr/lib64/libgobject-2.0.so.0.6000.6)
 |    by 0x135E5D: virt_viewer_session_spice_main_channel_event (virt-viewer-session-spice.c:699)
 |    by 0x564D741: g_closure_invoke (in /usr/lib64/libgobject-2.0.so.0.6000.6)
 |    by 0x56614F3: ??? (in /usr/lib64/libgobject-2.0.so.0.6000.6)
 |    by 0x566A34D: g_signal_emit_valist (in /usr/lib64/libgobject-2.0.so.0.6000.6)
 |    by 0x53149E3: emit_main_context (gio-coroutine.c:198)
 |  Block was alloc'd at
 |    at 0x483880B: malloc (vg_replace_malloc.c:309)
 |    by 0x56EBC98: g_malloc (in /usr/lib64/libglib-2.0.so.0.6000.6)
 |    by 0x5705C43: g_strdup (in /usr/lib64/libglib-2.0.so.0.6000.6)
 |    by 0x11EB80: remote_viewer_initial_connect (remote-viewer.c:696)
 |    by 0x11EB80: remote_viewer_start (remote-viewer.c:790)
 |    by 0x1250D3: virt_viewer_app_start (virt-viewer-app.c:1727)
 |    by 0x127108: virt_viewer_app_on_application_startup (virt-viewer-app.c:1870)
 |    by 0x564D741: g_closure_invoke (in /usr/lib64/libgobject-2.0.so.0.6000.6)
 |    by 0x5661638: ??? (in /usr/lib64/libgobject-2.0.so.0.6000.6)
 |    by 0x566A34D: g_signal_emit_valist (in /usr/lib64/libgobject-2.0.so.0.6000.6)
 |    by 0x566A972: g_signal_emit (in /usr/lib64/libgobject-2.0.so.0.6000.6)
 |    by 0x553ECA1: g_application_register (in /usr/lib64/libgio-2.0.so.0.6000.6)
 |    by 0x553F41D: g_application_run (in /usr/lib64/libgio-2.0.so.0.6000.6)

Signed-off-by: Victor Toso <victortoso at redhat.com>
---
 src/remote-viewer.c | 9 ++++-----
 1 file changed, 4 insertions(+), 5 deletions(-)

diff --git a/src/remote-viewer.c b/src/remote-viewer.c
index 8938ef9..f83ff6d 100644
--- a/src/remote-viewer.c
+++ b/src/remote-viewer.c
@@ -645,17 +645,16 @@ remote_viewer_recent_add(gchar *uri, const gchar *mime_type)
 
 static void
 remote_viewer_session_connected(VirtViewerSession *session,
-                                gchar *guri)
+                                VirtViewerApp *app)
 {
     gchar *uri = virt_viewer_session_get_uri(session);
     const gchar *mime = virt_viewer_session_mime_type(session);
 
     if (uri == NULL)
-        uri = g_strdup(guri);
+        g_object_get(app, "guri", &uri, NULL);
 
     remote_viewer_recent_add(uri, mime);
     g_free(uri);
-    g_free(guri);
 }
 
 static gchar *
@@ -675,7 +674,7 @@ read_all_stdin(gsize *len, GError **err)
 }
 
 static gboolean
-remote_viewer_initial_connect(RemoteViewer *self, const gchar *type, const gchar *guri,
+remote_viewer_initial_connect(RemoteViewer *self, const gchar *type, const gchar *guri G_GNUC_UNUSED,
                               VirtViewerFile *vvfile, GError **error)
 {
     VirtViewerApp *app = VIRT_VIEWER_APP(self);
@@ -694,7 +693,7 @@ remote_viewer_initial_connect(RemoteViewer *self, const gchar *type, const gchar
     }
 
     g_signal_connect(virt_viewer_app_get_session(app), "session-connected",
-                     G_CALLBACK(remote_viewer_session_connected), g_strdup(guri));
+                     G_CALLBACK(remote_viewer_session_connected), app);
 
     virt_viewer_session_set_file(virt_viewer_app_get_session(app), vvfile);
 #ifdef HAVE_OVIRT
-- 
2.21.0