[Spice-devel] [PATCH spice-server 2/2] Fix crash attempting to connect duplicate channels

[Frediano Ziglio]

You could easily trigger this issue using multiple monitors and a
modified spice-gtk client with this patch:

  --- a/src/channel-main.c
  +++ b/src/channel-main.c
  @@ -1699,6 +1699,7 @@ static gboolean _channel_new(channel_new_t *c)
   {
       g_return_val_if_fail(c != NULL, FALSE);

  +    if (c->type == SPICE_CHANNEL_DISPLAY) c->id = 0;
       spice_channel_new(c->session, c->type, c->id);

       g_object_unref(c->session);

which cause a crash like

(process:28742): Spice-WARNING **: Failed to create channel client: Client 0x40246f5d0: duplicate channel type 2 id 0
2017-08-24 09:36:57.451+0000: shutting down, reason=crashed

RedChannelClient is an GInitable type, which means that the object is
constructed, and then the _init() function is called, which can fail.
If the _init() fails, the newly-created object will be destroyed. As
part of _init(), we add a new watch for the stream using the core
interface that is associated with the channel. After adding the watch,
our rcc creation fails (due to duplicate ID), and the rcc object is
unreffed. This results in a call to reds_stream_free() (since the rcc
now owns the stream). But in reds_stream_free, we were trying to remove
the watch from the core interface associated with the RedsState. For
most channels, these two core interfaces are equivalent. But for the
Display and Cursor channels, it is the core Glib-based interface
associated with the RedWorker.

The watch in RedsStream by default is bound to the Qemu provided
SpiceCoreInterface while RedChannelClient bound it to Glib one causing
the crash when the watch is deleted from RedsStream. Change the bound
interface.

Signed-off-by: Frediano Ziglio <fziglio at redhat.com>
---
Changes since v2:
- improved commit message (Jonathon)
---
 server/red-channel-client.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/server/red-channel-client.c b/server/red-channel-client.c
index 655e41c73..35bb8d922 100644
--- a/server/red-channel-client.c
+++ b/server/red-channel-client.c
@@ -339,6 +339,16 @@ red_channel_client_finalize(GObject *object)
 {
     RedChannelClient *self = RED_CHANNEL_CLIENT(object);
 
+    SpiceCoreInterfaceInternal *core = red_channel_get_core_interface(self->priv->channel);
+    if (self->priv->latency_monitor.timer) {
+        core->timer_remove(core, self->priv->latency_monitor.timer);
+        self->priv->latency_monitor.timer = NULL;
+    }
+    if (self->priv->connectivity_monitor.timer) {
+        core->timer_remove(core, self->priv->connectivity_monitor.timer);
+        self->priv->connectivity_monitor.timer = NULL;
+    }
+
     reds_stream_free(self->priv->stream);
     self->priv->stream = NULL;
 
@@ -921,12 +931,14 @@ static gboolean red_channel_client_initable_init(GInitable *initable,
     }
 
     core = red_channel_get_core_interface(self->priv->channel);
-    if (self->priv->stream)
+    if (self->priv->stream) {
+        reds_stream_set_core_interface(self->priv->stream, core);
         self->priv->stream->watch =
             core->watch_add(core, self->priv->stream->socket,
                             SPICE_WATCH_EVENT_READ,
                             red_channel_client_event,
                             self);
+    }
 
     if (self->priv->monitor_latency
         && reds_stream_get_family(self->priv->stream) != AF_UNIX) {
-- 
2.13.5