diff --git a/SOURCES/0002-lz-Avoid-buffer-reading-overflow-checking-for-image-.patch b/SOURCES/0002-lz-Avoid-buffer-reading-overflow-checking-for-image-.patch new file mode 100644 index 0000000..2ecfa91 --- /dev/null +++ b/SOURCES/0002-lz-Avoid-buffer-reading-overflow-checking-for-image-.patch @@ -0,0 +1,29 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Frediano Ziglio +Date: Fri, 22 Dec 2017 18:43:00 +0000 +Subject: [PATCH spice-common 1/2] lz: Avoid buffer reading overflow checking + for image type + +The type of the image is just copied from network without +any check and later used for array indexing. + +Signed-off-by: Frediano Ziglio +Acked-by: Uri Lublin +--- + common/lz.c | 3 +++ + 1 file changed, 3 insertions(+) + +diff --git a/spice-common/common/lz.c b/spice-common/common/lz.c +index 87c13db..2c5d5e2 100644 +--- a/spice-common/common/lz.c ++++ b/spice-common/common/lz.c +@@ -593,6 +593,9 @@ void lz_decode_begin(LzContext *lz, uint8_t *io_ptr, unsigned int num_io_bytes, + } + + encoder->type = (LzImageType)decode_32(encoder); ++ if (encoder->type <= LZ_IMAGE_TYPE_INVALID || encoder->type > LZ_IMAGE_TYPE_A8) { ++ encoder->usr->error(encoder->usr, "invalid lz type %d\n", encoder->type); ++ } + encoder->width = decode_32(encoder); + encoder->height = decode_32(encoder); + encoder->stride = decode_32(encoder); diff --git a/SOURCES/0003-lz-More-checks-on-image-sizes.patch b/SOURCES/0003-lz-More-checks-on-image-sizes.patch new file mode 100644 index 0000000..422c9d7 --- /dev/null +++ b/SOURCES/0003-lz-More-checks-on-image-sizes.patch @@ -0,0 +1,121 @@ +From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 +From: Frediano Ziglio +Date: Mon, 25 Jun 2018 14:16:10 +0100 +Subject: [PATCH spice-common 2/2] lz: More checks on image sizes + +Extend sizes check also to decoding, actually the source data +decoding images should be less safe than encoding. +This avoids different integer overflows and buffer overflows. +To avoid potential issues images are limited to 1GB. + +Signed-off-by: Frediano Ziglio +Acked-by: Christophe Fergeau +--- + common/lz.c | 68 ++++++++++++++++++++++++++++++++++++----------------- + 1 file changed, 46 insertions(+), 22 deletions(-) + +diff --git a/spice-common/common/lz.c b/spice-common/common/lz.c +index 2c5d5e2..167e118 100644 +--- a/spice-common/common/lz.c ++++ b/spice-common/common/lz.c +@@ -53,6 +53,8 @@ + #define HASH_SIZE (1 << HASH_LOG) + #define HASH_MASK (HASH_SIZE - 1) + ++/* Maximum image size, mainly to avoid possible integer overflows */ ++#define SPICE_MAX_IMAGE_SIZE (1024 * 1024 * 1024 - 1) + + typedef struct LzImageSegment LzImageSegment; + struct LzImageSegment { +@@ -481,33 +483,53 @@ typedef uint16_t rgb16_pixel_t; + #undef LZ_UNEXPECT_CONDITIONAL + #undef LZ_EXPECT_CONDITIONAL + +-int lz_encode(LzContext *lz, LzImageType type, int width, int height, int top_down, +- uint8_t *lines, unsigned int num_lines, int stride, +- uint8_t *io_ptr, unsigned int num_io_bytes) ++static void lz_set_sizes(Encoder *encoder, int type, int width, int height, int stride) + { +- Encoder *encoder = (Encoder *)lz; +- uint8_t *io_ptr_end = io_ptr + num_io_bytes; +- +- encoder->type = type; +- encoder->width = width; +- encoder->height = height; +- encoder->stride = stride; ++ if (width < 0) { ++ encoder->usr->error(encoder->usr, "invalid lz width %d\n", width); ++ } ++ if (height < 0) { ++ encoder->usr->error(encoder->usr, "invalid lz height %d\n", height); ++ } ++ if (stride < 0) { ++ encoder->usr->error(encoder->usr, "invalid lz stride %d\n", stride); ++ } + +- if (IS_IMAGE_TYPE_PLT[encoder->type]) { +- if (encoder->stride > (width / PLT_PIXELS_PER_BYTE[encoder->type])) { +- if (((width % PLT_PIXELS_PER_BYTE[encoder->type]) == 0) || ( +- (encoder->stride - (width / PLT_PIXELS_PER_BYTE[encoder->type])) > 1)) { ++ if (IS_IMAGE_TYPE_PLT[type]) { ++ if (stride > (width / PLT_PIXELS_PER_BYTE[type])) { ++ if (((width % PLT_PIXELS_PER_BYTE[type]) == 0) || ( ++ (stride - (width / PLT_PIXELS_PER_BYTE[type])) > 1)) { + encoder->usr->error(encoder->usr, "stride overflows (plt)\n"); + } + } + } else { +- if (encoder->stride != width * RGB_BYTES_PER_PIXEL[encoder->type]) { ++ if (stride != width * RGB_BYTES_PER_PIXEL[type]) { + encoder->usr->error(encoder->usr, "stride != width*bytes_per_pixel (rgb) %d != %d * %d (%d)\n", +- encoder->stride, width, RGB_BYTES_PER_PIXEL[encoder->type], +- encoder->type); ++ stride, width, RGB_BYTES_PER_PIXEL[type], ++ type); + } + } + ++ // avoid too big images ++ if ((uint64_t) stride * height > SPICE_MAX_IMAGE_SIZE) { ++ encoder->usr->error(encoder->usr, "image too large\n"); ++ } ++ ++ encoder->type = type; ++ encoder->width = width; ++ encoder->height = height; ++ encoder->stride = stride; ++} ++ ++int lz_encode(LzContext *lz, LzImageType type, int width, int height, int top_down, ++ uint8_t *lines, unsigned int num_lines, int stride, ++ uint8_t *io_ptr, unsigned int num_io_bytes) ++{ ++ Encoder *encoder = (Encoder *)lz; ++ uint8_t *io_ptr_end = io_ptr + num_io_bytes; ++ ++ lz_set_sizes(encoder, type, width, height, stride); ++ + // assign the output buffer + if (!encoder_reset(encoder, io_ptr, io_ptr_end)) { + encoder->usr->error(encoder->usr, "lz encoder io reset failed\n"); +@@ -592,13 +614,15 @@ void lz_decode_begin(LzContext *lz, uint8_t *io_ptr, unsigned int num_io_bytes, + encoder->usr->error(encoder->usr, "bad version\n"); + } + +- encoder->type = (LzImageType)decode_32(encoder); +- if (encoder->type <= LZ_IMAGE_TYPE_INVALID || encoder->type > LZ_IMAGE_TYPE_A8) { ++ int type = decode_32(encoder); ++ if (type <= LZ_IMAGE_TYPE_INVALID || type > LZ_IMAGE_TYPE_A8) { + encoder->usr->error(encoder->usr, "invalid lz type %d\n", encoder->type); + } +- encoder->width = decode_32(encoder); +- encoder->height = decode_32(encoder); +- encoder->stride = decode_32(encoder); ++ int width = decode_32(encoder); ++ int height = decode_32(encoder); ++ int stride = decode_32(encoder); ++ lz_set_sizes(encoder, type, width, height, stride); ++ + *out_top_down = decode_32(encoder); + + *out_width = encoder->width; diff --git a/SOURCES/0004-spice-channel-Properly-error-out-if-reconnect-fails.patch b/SOURCES/0004-spice-channel-Properly-error-out-if-reconnect-fails.patch new file mode 100644 index 0000000..871e90d --- /dev/null +++ b/SOURCES/0004-spice-channel-Properly-error-out-if-reconnect-fails.patch @@ -0,0 +1,47 @@ +From 1ef54bc33a710162b69aa9d1a9505ae1544dbedd Mon Sep 17 00:00:00 2001 +From: Victor Toso +Date: Thu, 6 Sep 2018 14:04:53 +0200 +Subject: [PATCH] spice-channel: Properly error out if reconnect fails + +The channel_connect() function could fail leading to a spice-channel +existing as zombie (its coroutine return soon after). + +Check if channel_connect() fails and give a proper error signal to +user when that happens. + +Related: https://bugzilla.redhat.com/show_bug.cgi?id=1625550 + +Signed-off-by: Victor Toso +Acked-by: Frediano Ziglio +(cherry picked from commit 879926622d764a02b43a9147fb2a976765385115) +--- + src/spice-channel.c | 12 ++++++++---- + 1 file changed, 8 insertions(+), 4 deletions(-) + +diff --git a/src/spice-channel.c b/src/spice-channel.c +index 6f3ca27..2eec4e0 100644 +--- a/src/spice-channel.c ++++ b/src/spice-channel.c +@@ -2674,11 +2674,15 @@ cleanup: + if (c->state == SPICE_CHANNEL_STATE_RECONNECTING || + c->state == SPICE_CHANNEL_STATE_SWITCHING) { + g_warn_if_fail(c->event == SPICE_CHANNEL_NONE); +- channel_connect(channel, c->tls); +- g_object_unref(channel); +- } else +- g_idle_add(spice_channel_delayed_unref, data); ++ if (channel_connect(channel, c->tls)) { ++ g_object_unref(channel); ++ return NULL; ++ } ++ ++ c->event = SPICE_CHANNEL_ERROR_CONNECT; ++ } + ++ g_idle_add(spice_channel_delayed_unref, channel); + /* Co-routine exits now - the SpiceChannel object may no longer exist, + so don't do anything else now unless you like SEGVs */ + return NULL; +-- +2.20.1 + diff --git a/SOURCES/0005-usb-device-manager-Handle-connectionless-channel.patch b/SOURCES/0005-usb-device-manager-Handle-connectionless-channel.patch new file mode 100644 index 0000000..8e9ddc8 --- /dev/null +++ b/SOURCES/0005-usb-device-manager-Handle-connectionless-channel.patch @@ -0,0 +1,77 @@ +From c90ceeab44a7c5db9c35965c3f7c3ec245fab65f Mon Sep 17 00:00:00 2001 +From: Victor Toso +Date: Thu, 6 Sep 2018 15:37:46 +0200 +Subject: [PATCH] usb-device-manager: Handle connectionless channel + +As we are not able to redirect anything in case that usbredir channel +is not connected. + +Related: https://bugzilla.redhat.com/show_bug.cgi?id=1625550 + +Signed-off-by: Victor Toso +Acked-by: Frediano Ziglio +(cherry picked from commit bd195d3f76f115b5ef5d1ad0e83e9ccea5a04d18) +--- + src/usb-device-manager.c | 32 ++++++++++++++++++++++++++++++++ + 1 file changed, 32 insertions(+) + +diff --git a/src/usb-device-manager.c b/src/usb-device-manager.c +index 35b1eb7..8ac5fda 100644 +--- a/src/usb-device-manager.c ++++ b/src/usb-device-manager.c +@@ -158,6 +158,8 @@ static void channel_new(SpiceSession *session, SpiceChannel *channel, + gpointer user_data); + static void channel_destroy(SpiceSession *session, SpiceChannel *channel, + gpointer user_data); ++static void channel_event(SpiceChannel *channel, SpiceChannelEvent event, ++ gpointer user_data); + #ifdef USE_GUDEV + static void spice_usb_device_manager_uevent_cb(GUdevClient *client, + const gchar *action, +@@ -843,6 +845,8 @@ static void channel_new(SpiceSession *session, SpiceChannel *channel, + spice_channel_connect(channel); + g_ptr_array_add(self->priv->channels, channel); + ++ g_signal_connect(channel, "channel-event", G_CALLBACK(channel_event), self); ++ + spice_usb_device_manager_check_redir_on_connect(self, channel); + + /* +@@ -865,6 +869,34 @@ static void channel_destroy(SpiceSession *session, SpiceChannel *channel, + g_ptr_array_remove(self->priv->channels, channel); + } + ++static void channel_event(SpiceChannel *channel, SpiceChannelEvent event, ++ gpointer user_data) ++ ++{ ++ SpiceUsbDeviceManager *self = user_data; ++ ++ switch (event) { ++ case SPICE_CHANNEL_NONE: ++ case SPICE_CHANNEL_OPENED: ++ return; ++ ++ case SPICE_CHANNEL_SWITCHING: ++ case SPICE_CHANNEL_CLOSED: ++ case SPICE_CHANNEL_ERROR_CONNECT: ++ case SPICE_CHANNEL_ERROR_TLS: ++ case SPICE_CHANNEL_ERROR_LINK: ++ case SPICE_CHANNEL_ERROR_AUTH: ++ case SPICE_CHANNEL_ERROR_IO: ++ g_signal_handlers_disconnect_by_func(channel, channel_event, user_data); ++ g_ptr_array_remove(self->priv->channels, channel); ++ return; ++ default: ++ g_warning("Unhandled SpiceChannelEvent %d, disconnecting usbredir %p", event, channel); ++ g_signal_handlers_disconnect_by_func(channel, channel_event, user_data); ++ g_ptr_array_remove(self->priv->channels, channel); ++ } ++} ++ + static void spice_usb_device_manager_auto_connect_cb(GObject *gobject, + GAsyncResult *res, + gpointer user_data) +-- +2.20.1 + diff --git a/SOURCES/0006-channel-usbredir-Add-FIXMEs-on-channel-reset-issues.patch b/SOURCES/0006-channel-usbredir-Add-FIXMEs-on-channel-reset-issues.patch new file mode 100644 index 0000000..0c844c3 --- /dev/null +++ b/SOURCES/0006-channel-usbredir-Add-FIXMEs-on-channel-reset-issues.patch @@ -0,0 +1,67 @@ +From 21d6d9921466eb06a8f7120da5f5592f8216fccb Mon Sep 17 00:00:00 2001 +From: Victor Toso +Date: Wed, 5 Sep 2018 15:39:56 +0200 +Subject: [PATCH] channel-usbredir: Add FIXMEs on channel-reset issues + +This should not change code behavior, only add some comments. + +This is a preparatory patch for bug below, introduced with: + + commit 9fbf679453d8dbfe797a738cb536136599d7adab + Author: Kirill Moizik + Date: Tue Mar 8 16:05:57 2016 +0200 + + usbredir: Disconnect USB device asynchronously + +Related: https://bugzilla.redhat.com/show_bug.cgi?id=1625550 + +Signed-off-by: Victor Toso +Acked-by: Frediano Ziglio +(cherry picked from commit f1aee52b098440273f749042bde23d61b73e4da5) +--- + src/channel-usbredir.c | 27 +++++++++++++++++++-------- + 1 file changed, 19 insertions(+), 8 deletions(-) + +diff --git a/src/channel-usbredir.c b/src/channel-usbredir.c +index 6ffe546..13927cc 100644 +--- a/src/channel-usbredir.c ++++ b/src/channel-usbredir.c +@@ -158,16 +158,27 @@ static void spice_usbredir_channel_reset(SpiceChannel *c, gboolean migrating) + SpiceUsbredirChannel *channel = SPICE_USBREDIR_CHANNEL(c); + SpiceUsbredirChannelPrivate *priv = channel->priv; + +- if (priv->host) { +- if (priv->state == STATE_CONNECTED) { +- spice_usbredir_channel_disconnect_device_async(channel, NULL, +- _channel_reset_cb, GUINT_TO_POINTER(migrating)); +- } else { +- _channel_reset_finish(channel); +- } +- } else { ++ /* Host isn't running, just reset */ ++ if (!priv->host) { + SPICE_CHANNEL_CLASS(spice_usbredir_channel_parent_class)->channel_reset(c, migrating); ++ return; + } ++ ++ /* Host is running, so we might need to disconnect the usb devices async. ++ * This should not block channel_reset() otherwise we might run in reconnection ++ * problems such as https://bugzilla.redhat.com/show_bug.cgi?id=1625550 ++ * No operation from here on should rely on SpiceChannel as its coroutine ++ * might be terminated. */ ++ ++ if (priv->state == STATE_CONNECTED) { ++ /* FIXME: We should chain-up parent's channel-reset here */ ++ spice_usbredir_channel_disconnect_device_async(channel, NULL, ++ _channel_reset_cb, GUINT_TO_POINTER(migrating)); ++ return; ++ } ++ ++ /* FIXME: This does not chain-up with parent's channel-reset, which is a must */ ++ _channel_reset_finish(channel); + } + #endif + +-- +2.20.1 + diff --git a/SOURCES/0007-channel-usbredir-Chain-up-with-parent-s-channel-rese.patch b/SOURCES/0007-channel-usbredir-Chain-up-with-parent-s-channel-rese.patch new file mode 100644 index 0000000..23d25b6 --- /dev/null +++ b/SOURCES/0007-channel-usbredir-Chain-up-with-parent-s-channel-rese.patch @@ -0,0 +1,66 @@ +From 5132d4672c65129841869da897d087c721beab28 Mon Sep 17 00:00:00 2001 +From: Victor Toso +Date: Wed, 5 Sep 2018 15:43:28 +0200 +Subject: [PATCH] channel-usbredir: Chain-up with parent's channel-reset + +Otherwise spice-channel is left with a broken state. + +This code moves parent's call to channel_reset() into +_channel_reset_finish() - Note that spice-channel's channel_reset() +can be called from GMainContext. + +Resolves: https://bugzilla.redhat.com/show_bug.cgi?id=1625550 + +Signed-off-by: Victor Toso +Acked-by: Frediano Ziglio +(cherry picked from commit 3afee7ae8db387e7f4339f2a4afaa63babf358b7) +--- + src/channel-usbredir.c | 11 +++++------ + 1 file changed, 5 insertions(+), 6 deletions(-) + +diff --git a/src/channel-usbredir.c b/src/channel-usbredir.c +index 13927cc..870938b 100644 +--- a/src/channel-usbredir.c ++++ b/src/channel-usbredir.c +@@ -122,7 +122,7 @@ static void spice_usbredir_channel_init(SpiceUsbredirChannel *channel) + + #ifdef USE_USBREDIR + +-static void _channel_reset_finish(SpiceUsbredirChannel *channel) ++static void _channel_reset_finish(SpiceUsbredirChannel *channel, gboolean migrating) + { + SpiceUsbredirChannelPrivate *priv = channel->priv; + +@@ -135,6 +135,8 @@ static void _channel_reset_finish(SpiceUsbredirChannel *channel) + spice_usbredir_channel_set_context(channel, priv->context); + + spice_usbredir_channel_unlock(channel); ++ ++ SPICE_CHANNEL_CLASS(spice_usbredir_channel_parent_class)->channel_reset(SPICE_CHANNEL(channel), migrating); + } + + static void _channel_reset_cb(GObject *gobject, +@@ -146,9 +148,7 @@ static void _channel_reset_cb(GObject *gobject, + gboolean migrating = GPOINTER_TO_UINT(user_data); + GError *err = NULL; + +- _channel_reset_finish(channel); +- +- SPICE_CHANNEL_CLASS(spice_usbredir_channel_parent_class)->channel_reset(spice_channel, migrating); ++ _channel_reset_finish(channel, migrating); + + spice_usbredir_channel_disconnect_device_finish(channel, result, &err); + } +@@ -177,8 +177,7 @@ static void spice_usbredir_channel_reset(SpiceChannel *c, gboolean migrating) + return; + } + +- /* FIXME: This does not chain-up with parent's channel-reset, which is a must */ +- _channel_reset_finish(channel); ++ _channel_reset_finish(channel, migrating); + } + #endif + +-- +2.20.1 + diff --git a/SPECS/spice-gtk.spec b/SPECS/spice-gtk.spec index 5018a13..28dd743 100644 --- a/SPECS/spice-gtk.spec +++ b/SPECS/spice-gtk.spec @@ -4,7 +4,7 @@ Name: spice-gtk Version: 0.35 -Release: 2%{?dist} +Release: 4%{?dist} Summary: A GTK+ widget for SPICE clients Group: System Environment/Libraries @@ -13,6 +13,12 @@ URL: https://www.spice-space.org/ Source0: https://www.spice-space.org/download/gtk/%{name}-%{version}%{?_version_suffix}.tar.bz2 Patch0001: 0001-Fix-flexible-array-buffer-overflow.patch +Patch0002: 0002-lz-Avoid-buffer-reading-overflow-checking-for-image-.patch +Patch0003: 0003-lz-More-checks-on-image-sizes.patch +Patch0004: 0004-spice-channel-Properly-error-out-if-reconnect-fails.patch +Patch0005: 0005-usb-device-manager-Handle-connectionless-channel.patch +Patch0006: 0006-channel-usbredir-Add-FIXMEs-on-channel-reset-issues.patch +Patch0007: 0007-channel-usbredir-Chain-up-with-parent-s-channel-rese.patch BuildRequires: intltool BuildRequires: usbredir-devel >= 0.6-8 @@ -115,6 +121,12 @@ spicy-screenshot is a tool to capture screen-shots of a SPICE desktop. %setup -q -n spice-gtk-%{version}%{?_version_suffix} %patch0001 -p1 +%patch0002 -p1 +%patch0003 -p1 +%patch0004 -p1 +%patch0005 -p1 +%patch0006 -p1 +%patch0007 -p1 %build %configure \ @@ -187,6 +199,14 @@ rm -f %{buildroot}%{_libdir}/*.la %{_bindir}/spicy-stats %changelog +* Fri Mar 15 2019 Victor Toso - 0.35-4 +- Fix bad channel-reset on usbredir + Resolves: rhbz#1625550 + +* Wed Oct 10 2018 Frediano Ziglio - 0.35-3 +- Fix insufficient encoding checks for LZ + Resolves: rhbz#1598652 + * Fri Aug 10 2018 Frediano Ziglio - 0.35-2 - Fix flexible array buffer overflow Resolves: rhbz#1596008