From a13173ae649412d06106a0d9c6d29e6a45d5bf57 Mon Sep 17 00:00:00 2001 From: Victor Toso Date: Oct 04 2019 08:02:42 +0000 Subject: remote-viewer: fix free on dangling pointer 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. A fix trying to use VirtViewerApp URI avoid the crash but introduces regression while running remote-viewer with ovirt so, keeping the changes to a minimum to avoid it, just use g_intern_string() for now. 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 Acked-by: Eduardo Lima (Etrunko) --- diff --git a/src/remote-viewer.c b/src/remote-viewer.c index 8938ef9..8eaa72e 100644 --- a/src/remote-viewer.c +++ b/src/remote-viewer.c @@ -645,17 +645,13 @@ remote_viewer_recent_add(gchar *uri, const gchar *mime_type) static void remote_viewer_session_connected(VirtViewerSession *session, - gchar *guri) + const gchar *guri) { gchar *uri = virt_viewer_session_get_uri(session); const gchar *mime = virt_viewer_session_mime_type(session); - if (uri == NULL) - uri = g_strdup(guri); - - remote_viewer_recent_add(uri, mime); + remote_viewer_recent_add(uri != NULL ? uri : (char *) guri, mime); g_free(uri); - g_free(guri); } static gchar * @@ -694,7 +690,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), (gpointer) g_intern_string(guri)); virt_viewer_session_set_file(virt_viewer_app_get_session(app), vvfile); #ifdef HAVE_OVIRT