[Spice-devel] [PATCH spice-gtk 1/2] usbredir: Check for existing usb channels after libusb init

[Hans de Goede]

Currently trying to view a usbredir enabled vm from virt-manager causes
virt-manager to crash. This crash is caused by the following happening:

-virt-manager sets up the session, including connecting all the channels
-a spice-gtk internal code path calls spice_usb_device_manager_get()
-spice_usb_device_manager_get calls channel_new on all already connected
 usb channels
-channel_new does:
 spice_usbredir_channel_set_context(SPICE_USBREDIR_CHANNEL(channel),
                                    self->priv->context);
-But self->priv->context has not been set yet (so is NULL) -> segfault!

This patch fixes this by moving the iterating over already connected
usb channels to after the setting of self->priv->context. Note this means
that the channels will no longer get checked when there is no USB_REDIR
support. That is not a problem since spice_usb_device_manager_initable_init
will return FALSE in that case anyways.

Signed-off-by: Hans de Goede <hdegoede at redhat.com>
---
 gtk/usb-device-manager.c |   45 +++++++++++++++++++++------------------------
 1 file changed, 21 insertions(+), 24 deletions(-)

diff --git a/gtk/usb-device-manager.c b/gtk/usb-device-manager.c
index 29e9b4c..14b60c9 100644
--- a/gtk/usb-device-manager.c
+++ b/gtk/usb-device-manager.c
@@ -62,13 +62,6 @@
  */
 
 /* ------------------------------------------------------------------ */
-/* Prototypes for private functions */
-static void channel_new(SpiceSession *session, SpiceChannel *channel,
-                        gpointer user_data);
-static void channel_destroy(SpiceSession *session, SpiceChannel *channel,
-                            gpointer user_data);
-
-/* ------------------------------------------------------------------ */
 /* gobject glue                                                       */
 
 #define SPICE_USB_DEVICE_MANAGER_GET_PRIVATE(obj)                                  \
@@ -109,6 +102,10 @@ struct _SpiceUsbDeviceManagerPrivate {
 };
 
 #ifdef USE_USBREDIR
+static void channel_new(SpiceSession *session, SpiceChannel *channel,
+                        gpointer user_data);
+static void channel_destroy(SpiceSession *session, SpiceChannel *channel,
+                            gpointer user_data);
 static void spice_usb_device_manager_uevent_cb(GUdevClient     *client,
                                                const gchar     *action,
                                                GUdevDevice     *udevice,
@@ -148,11 +145,11 @@ static gboolean spice_usb_device_manager_initable_init(GInitable  *initable,
                                                     GCancellable  *cancellable,
                                                     GError        **err)
 {
-    GList *list;
-    GList *it;
     SpiceUsbDeviceManager *self;
     SpiceUsbDeviceManagerPrivate *priv;
 #ifdef USE_USBREDIR
+    GList *list;
+    GList *it;
     int rc;
     const gchar *const subsystems[] = {"usb", NULL};
 #endif
@@ -175,17 +172,8 @@ static gboolean spice_usb_device_manager_initable_init(GInitable  *initable,
         return FALSE;
     }
 
-    g_signal_connect(priv->session, "channel-new",
-                     G_CALLBACK(channel_new), self);
-    g_signal_connect(priv->session, "channel-destroy",
-                     G_CALLBACK(channel_destroy), self);
-    list = spice_session_get_channels(priv->session);
-    for (it = g_list_first(list); it != NULL; it = g_list_next(it)) {
-        channel_new(priv->session, it->data, (gpointer*)self);
-    }
-    g_list_free(list);
-
 #ifdef USE_USBREDIR
+    /* Initialize libusb */
     rc = libusb_init(&priv->context);
     if (rc < 0) {
         const char *desc = spice_usbutil_libusb_strerror(rc);
@@ -195,6 +183,18 @@ static gboolean spice_usb_device_manager_initable_init(GInitable  *initable,
         return FALSE;
     }
 
+    /* Start listening for usb channels connect/disconnect */
+    g_signal_connect(priv->session, "channel-new",
+                     G_CALLBACK(channel_new), self);
+    g_signal_connect(priv->session, "channel-destroy",
+                     G_CALLBACK(channel_destroy), self);
+    list = spice_session_get_channels(priv->session);
+    for (it = g_list_first(list); it != NULL; it = g_list_next(it)) {
+        channel_new(priv->session, it->data, (gpointer*)self);
+    }
+    g_list_free(list);
+
+    /* Start listening for usb devices plug / unplug */
     priv->udev = g_udev_client_new(subsystems);
     g_signal_connect(G_OBJECT(priv->udev), "uevent",
                      G_CALLBACK(spice_usb_device_manager_uevent_cb), self);
@@ -460,10 +460,11 @@ static void spice_usb_device_manager_class_init(SpiceUsbDeviceManagerClass *klas
     g_type_class_add_private(klass, sizeof(SpiceUsbDeviceManagerPrivate));
 }
 
+#ifdef USE_USBREDIR
+
 /* ------------------------------------------------------------------ */
 /* gudev / libusb Helper functions                                    */
 
-#ifdef USE_USBREDIR
 static gboolean spice_usb_device_manager_get_udev_bus_n_address(
     GUdevDevice *udev, int *bus, int *address)
 {
@@ -480,7 +481,6 @@ static gboolean spice_usb_device_manager_get_udev_bus_n_address(
 
     return *bus && *address;
 }
-#endif
 
 /* ------------------------------------------------------------------ */
 /* callbacks                                                          */
@@ -491,11 +491,9 @@ static void channel_new(SpiceSession *session, SpiceChannel *channel,
     SpiceUsbDeviceManager *self = user_data;
 
     if (SPICE_IS_USBREDIR_CHANNEL(channel)) {
-#ifdef USE_USBREDIR
         spice_usbredir_channel_set_context(SPICE_USBREDIR_CHANNEL(channel),
                                            self->priv->context);
         spice_channel_connect(channel);
-#endif
         g_ptr_array_add(self->priv->channels, channel);
     }
 }
@@ -509,7 +507,6 @@ static void channel_destroy(SpiceSession *session, SpiceChannel *channel,
         g_ptr_array_remove(self->priv->channels, channel);
 }
 
-#ifdef USE_USBREDIR
 static void spice_usb_device_manager_auto_connect_cb(GObject      *gobject,
                                                      GAsyncResult *res,
                                                      gpointer      user_data)
-- 
1.7.9.3