[PATCH spice-server 1/5] Fix crash attempting to connect duplicate channels

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



You could easily trigger this issue using multiple monitors and
a modified 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

One issue is that g_initable_new in this case returns NULL (dcc.c).

The other is that the watch in RedsStream is supposed to be bound
to the Qemu provided SpiceCoreInterface while RedChannelClient bound
it to Glib one causing the crash when the watch is deleted from
RedsStream. To avoid this a new watch is saved in
RedChannelClientPrivate.

Signed-off-by: Frediano Ziglio <fziglio@xxxxxxxxxx>
---
 server/dcc.c                |  4 +++-
 server/red-channel-client.c | 39 ++++++++++++++++++++++++++++-----------
 2 files changed, 31 insertions(+), 12 deletions(-)

diff --git a/server/dcc.c b/server/dcc.c
index 4490507b..2e1acd23 100644
--- a/server/dcc.c
+++ b/server/dcc.c
@@ -516,7 +516,9 @@ DisplayChannelClient *dcc_new(DisplayChannel *display,
                          NULL);
     spice_debug("New display (client %p) dcc %p stream %p", client, dcc, stream);
     common_graphics_channel_set_during_target_migrate(COMMON_GRAPHICS_CHANNEL(display), mig_target);
-    dcc->priv->id = common_graphics_channel_get_qxl(COMMON_GRAPHICS_CHANNEL(display))->id;
+    if (dcc) {
+        dcc->priv->id = common_graphics_channel_get_qxl(COMMON_GRAPHICS_CHANNEL(display))->id;
+    }
 
     return dcc;
 }
diff --git a/server/red-channel-client.c b/server/red-channel-client.c
index f1042dd4..1162ddca 100644
--- a/server/red-channel-client.c
+++ b/server/red-channel-client.c
@@ -120,6 +120,7 @@ struct RedChannelClientPrivate
     RedChannel *channel;
     RedClient  *client;
     RedsStream *stream;
+    SpiceWatch *watch;
     gboolean monitor_latency;
 
     struct {
@@ -339,6 +340,20 @@ 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->watch) {
+        core->watch_remove(core, self->priv->watch);
+        self->priv->watch = NULL;
+    }
+    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;
 
@@ -922,7 +937,7 @@ static gboolean red_channel_client_initable_init(GInitable *initable,
 
     core = red_channel_get_core_interface(self->priv->channel);
     if (self->priv->stream)
-        self->priv->stream->watch =
+        self->priv->watch =
             core->watch_add(core, self->priv->stream->socket,
                             SPICE_WATCH_EVENT_READ,
                             red_channel_client_event,
@@ -1003,9 +1018,11 @@ void red_channel_client_destroy(RedChannelClient *rcc)
 void red_channel_client_shutdown(RedChannelClient *rcc)
 {
     if (rcc->priv->stream && !rcc->priv->stream->shutdown) {
-        SpiceCoreInterfaceInternal *core = red_channel_get_core_interface(rcc->priv->channel);
-        core->watch_remove(core, rcc->priv->stream->watch);
-        rcc->priv->stream->watch = NULL;
+        if (rcc->priv->watch) {
+            SpiceCoreInterfaceInternal *core = red_channel_get_core_interface(rcc->priv->channel);
+            core->watch_remove(core, rcc->priv->watch);
+            rcc->priv->watch = NULL;
+        }
         shutdown(rcc->priv->stream->socket, SHUT_RDWR);
         rcc->priv->stream->shutdown = TRUE;
     }
@@ -1294,10 +1311,10 @@ void red_channel_client_push(RedChannelClient *rcc)
         red_channel_client_send_item(rcc, pipe_item);
     }
     if (red_channel_client_no_item_being_sent(rcc) && g_queue_is_empty(&rcc->priv->pipe)
-        && rcc->priv->stream->watch) {
+        && rcc->priv->watch) {
         SpiceCoreInterfaceInternal *core;
         core = red_channel_get_core_interface(rcc->priv->channel);
-        core->watch_update_mask(core, rcc->priv->stream->watch,
+        core->watch_update_mask(core, rcc->priv->watch,
                                 SPICE_WATCH_EVENT_READ);
     }
     rcc->priv->during_send = FALSE;
@@ -1511,10 +1528,10 @@ static inline gboolean prepare_pipe_add(RedChannelClient *rcc, RedPipeItem *item
         red_pipe_item_unref(item);
         return FALSE;
     }
-    if (g_queue_is_empty(&rcc->priv->pipe) && rcc->priv->stream->watch) {
+    if (g_queue_is_empty(&rcc->priv->pipe) && rcc->priv->watch) {
         SpiceCoreInterfaceInternal *core;
         core = red_channel_get_core_interface(rcc->priv->channel);
-        core->watch_update_mask(core, rcc->priv->stream->watch,
+        core->watch_update_mask(core, rcc->priv->watch,
                                 SPICE_WATCH_EVENT_READ | SPICE_WATCH_EVENT_WRITE);
     }
     return TRUE;
@@ -1678,9 +1695,9 @@ void red_channel_client_disconnect(RedChannelClient *rcc)
     spice_printerr("rcc=%p (channel=%p type=%d id=%d)", rcc, channel,
                    type, id);
     red_channel_client_pipe_clear(rcc);
-    if (rcc->priv->stream->watch) {
-        core->watch_remove(core, rcc->priv->stream->watch);
-        rcc->priv->stream->watch = NULL;
+    if (rcc->priv->watch) {
+        core->watch_remove(core, rcc->priv->watch);
+        rcc->priv->watch = NULL;
     }
     if (rcc->priv->latency_monitor.timer) {
         core->timer_remove(core, rcc->priv->latency_monitor.timer);
-- 
2.13.5

_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/spice-devel




[Index of Archives]     [Linux ARM Kernel]     [Linux ARM]     [Linux Omap]     [Fedora ARM]     [IETF Annouce]     [Security]     [Bugtraq]     [Linux]     [Linux OMAP]     [Linux MIPS]     [ECOS]     [Asterisk Internet PBX]     [Linux API]     [Monitors]