diff --git a/SOURCES/0038-Add-const-to-test_capability-first-argument.patch b/SOURCES/0038-Add-const-to-test_capability-first-argument.patch
new file mode 100644
index 0000000..7fd9526
--- /dev/null
+++ b/SOURCES/0038-Add-const-to-test_capability-first-argument.patch
@@ -0,0 +1,40 @@
+From 17fd07ff2fd287fff6cc866a308097e75cb968f5 Mon Sep 17 00:00:00 2001
+From: Christophe Fergeau <cfergeau@redhat.com>
+Date: Wed, 19 Mar 2014 18:17:32 +0100
+Subject: [PATCH 1/3] Add const to test_capability first argument
+
+We don't modify the capabilities content, so it can be marked as const.
+---
+ server/red_channel.c | 2 +-
+ server/red_channel.h | 2 +-
+ 2 files changed, 2 insertions(+), 2 deletions(-)
+
+diff --git a/server/red_channel.c b/server/red_channel.c
+index 5df8f14..449e628 100644
+--- a/server/red_channel.c
++++ b/server/red_channel.c
+@@ -1159,7 +1159,7 @@ void red_channel_register_client_cbs(RedChannel *channel, ClientCbs *client_cbs)
+ }
+ }
+
+-int test_capabilty(uint32_t *caps, int num_caps, uint32_t cap)
++int test_capabilty(const uint32_t *caps, int num_caps, uint32_t cap)
+ {
+ uint32_t index = cap / 32;
+ if (num_caps < index + 1) {
+diff --git a/server/red_channel.h b/server/red_channel.h
+index 9e54dce..58109d5 100644
+--- a/server/red_channel.h
++++ b/server/red_channel.h
+@@ -223,7 +223,7 @@ typedef struct RedChannelCapabilities {
+ uint32_t *caps;
+ } RedChannelCapabilities;
+
+-int test_capabilty(uint32_t *caps, int num_caps, uint32_t cap);
++int test_capabilty(const uint32_t *caps, int num_caps, uint32_t cap);
+
+ typedef struct RedChannelClientLatencyMonitor {
+ int state;
+--
+2.4.4
+
diff --git a/SOURCES/0038-Avoid-race-conditions-reading-monitor-configs-from-g.patch b/SOURCES/0038-Avoid-race-conditions-reading-monitor-configs-from-g.patch
deleted file mode 100644
index c909575..0000000
--- a/SOURCES/0038-Avoid-race-conditions-reading-monitor-configs-from-g.patch
+++ /dev/null
@@ -1,117 +0,0 @@
-From 524eef10c6c6c2f3f30be28c56b8f96adc7901f0 Mon Sep 17 00:00:00 2001
-From: Frediano Ziglio <fziglio@redhat.com>
-Date: Tue, 9 Jun 2015 08:50:46 +0100
-Subject: [PATCH] Avoid race conditions reading monitor configs from guest
-
-For security reasons do not assume guest do not change structures it
-pass to Qemu.
-Guest could change count field while Qemu is copying QXLMonitorsConfig
-structure leading to heap corruption.
-This patch avoid it reading count only once.
-
-Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
----
- server/red_worker.c | 46 ++++++++++++++++++++++++++++++++--------------
- 1 file changed, 32 insertions(+), 14 deletions(-)
-
-diff --git a/server/red_worker.c b/server/red_worker.c
-index 9e6a6ad..955cac2 100644
---- a/server/red_worker.c
-+++ b/server/red_worker.c
-@@ -11270,7 +11270,8 @@ static inline void red_monitors_config_item_add(DisplayChannelClient *dcc)
- }
-
- static void worker_update_monitors_config(RedWorker *worker,
-- QXLMonitorsConfig *dev_monitors_config)
-+ QXLMonitorsConfig *dev_monitors_config,
-+ uint16_t count, uint16_t max_allowed)
- {
- int heads_size;
- MonitorsConfig *monitors_config;
-@@ -11279,22 +11280,22 @@ static void worker_update_monitors_config(RedWorker *worker,
- monitors_config_decref(worker->monitors_config);
-
- spice_debug("monitors config %d(%d)",
-- dev_monitors_config->count,
-- dev_monitors_config->max_allowed);
-- for (i = 0; i < dev_monitors_config->count; i++) {
-+ count,
-+ max_allowed);
-+ for (i = 0; i < count; i++) {
- spice_debug("+%d+%d %dx%d",
- dev_monitors_config->heads[i].x,
- dev_monitors_config->heads[i].y,
- dev_monitors_config->heads[i].width,
- dev_monitors_config->heads[i].height);
- }
-- heads_size = dev_monitors_config->count * sizeof(QXLHead);
-+ heads_size = count * sizeof(QXLHead);
- worker->monitors_config = monitors_config =
- spice_malloc(sizeof(*monitors_config) + heads_size);
- monitors_config->refs = 1;
- monitors_config->worker = worker;
-- monitors_config->count = dev_monitors_config->count;
-- monitors_config->max_allowed = dev_monitors_config->max_allowed;
-+ monitors_config->count = count;
-+ monitors_config->max_allowed = max_allowed;
- memcpy(monitors_config->heads, dev_monitors_config->heads, heads_size);
- }
-
-@@ -11678,33 +11679,50 @@ void handle_dev_display_migrate(void *opaque, void *payload)
- red_migrate_display(worker, rcc);
- }
-
-+static inline uint32_t qxl_monitors_config_size(uint32_t heads)
-+{
-+ return sizeof(QXLMonitorsConfig) + sizeof(QXLHead) * heads;
-+}
-+
- static void handle_dev_monitors_config_async(void *opaque, void *payload)
- {
- RedWorkerMessageMonitorsConfigAsync *msg = payload;
- RedWorker *worker = opaque;
-- int min_size = sizeof(QXLMonitorsConfig) + sizeof(QXLHead);
- int error;
-+ uint16_t count, max_allowed;
- QXLMonitorsConfig *dev_monitors_config =
- (QXLMonitorsConfig*)get_virt(&worker->mem_slots, msg->monitors_config,
-- min_size, msg->group_id, &error);
-+ qxl_monitors_config_size(1),
-+ msg->group_id, &error);
-
- if (error) {
- /* TODO: raise guest bug (requires added QXL interface) */
- return;
- }
- worker->driver_cap_monitors_config = 1;
-- if (dev_monitors_config->count == 0) {
-+ count = dev_monitors_config->count;
-+ max_allowed = dev_monitors_config->max_allowed;
-+ if (count == 0) {
- spice_warning("ignoring an empty monitors config message from driver");
- return;
- }
-- if (dev_monitors_config->count > dev_monitors_config->max_allowed) {
-+ if (count > max_allowed) {
- spice_warning("ignoring malformed monitors_config from driver, "
- "count > max_allowed %d > %d",
-- dev_monitors_config->count,
-- dev_monitors_config->max_allowed);
-+ count,
-+ max_allowed);
- return;
- }
-- worker_update_monitors_config(worker, dev_monitors_config);
-+ /* get pointer again to check virtual size */
-+ dev_monitors_config =
-+ (QXLMonitorsConfig*)get_virt(&worker->mem_slots, msg->monitors_config,
-+ qxl_monitors_config_size(count),
-+ msg->group_id, &error);
-+ if (error) {
-+ /* TODO: raise guest bug (requires added QXL interface) */
-+ return;
-+ }
-+ worker_update_monitors_config(worker, dev_monitors_config, count, max_allowed);
- red_worker_push_monitors_config(worker);
- }
-
diff --git a/SOURCES/0039-Introduce-red_link_info_test_capability.patch b/SOURCES/0039-Introduce-red_link_info_test_capability.patch
new file mode 100644
index 0000000..8b17861
--- /dev/null
+++ b/SOURCES/0039-Introduce-red_link_info_test_capability.patch
@@ -0,0 +1,60 @@
+From 0b0ca2c972041b785145c558dfd0e8372869f696 Mon Sep 17 00:00:00 2001
+From: Christophe Fergeau <cfergeau@redhat.com>
+Date: Wed, 5 Mar 2014 11:59:37 +0100
+Subject: [PATCH 2/3] Introduce red_link_info_test_capability()
+
+This just hides a bit of pointer arithmetic away from reds_send_link_ack.
+This helper will be used in the next commits.
+---
+ server/reds.c | 21 ++++++++++++++++++---
+ 1 file changed, 18 insertions(+), 3 deletions(-)
+
+diff --git a/server/reds.c b/server/reds.c
+index ccba67c..c1edf67 100644
+--- a/server/reds.c
++++ b/server/reds.c
+@@ -1446,6 +1446,22 @@ static void reds_channel_init_auth_caps(RedLinkInfo *link, RedChannel *channel)
+ red_channel_set_common_cap(channel, SPICE_COMMON_CAP_PROTOCOL_AUTH_SELECTION);
+ }
+
++
++static const uint32_t *red_link_info_get_caps(const RedLinkInfo *link)
++{
++ const uint8_t *caps_start = (const uint8_t *)link->link_mess;
++
++ return (const uint32_t *)(caps_start + link->link_mess->caps_offset);
++}
++
++static bool red_link_info_test_capability(const RedLinkInfo *link, uint32_t cap)
++{
++ const uint32_t *caps = red_link_info_get_caps(link);
++
++ return test_capabilty(caps, link->link_mess->num_common_caps, cap);
++}
++
++
+ static int reds_send_link_ack(RedLinkInfo *link)
+ {
+ SpiceLinkHeader header;
+@@ -2701,7 +2717,6 @@ static void reds_handle_read_link_done(void *opaque)
+ SpiceLinkMess *link_mess = link->link_mess;
+ AsyncRead *obj = &link->asyc_read;
+ uint32_t num_caps = link_mess->num_common_caps + link_mess->num_channel_caps;
+- uint32_t *caps = (uint32_t *)((uint8_t *)link_mess + link_mess->caps_offset);
+ int auth_selection;
+
+ if (num_caps && (num_caps * sizeof(uint32_t) + link_mess->caps_offset >
+@@ -2712,8 +2727,8 @@ static void reds_handle_read_link_done(void *opaque)
+ return;
+ }
+
+- auth_selection = test_capabilty(caps, link_mess->num_common_caps,
+- SPICE_COMMON_CAP_PROTOCOL_AUTH_SELECTION);
++ auth_selection = red_link_info_test_capability(link,
++ SPICE_COMMON_CAP_PROTOCOL_AUTH_SELECTION);
+
+ if (!reds_security_check(link)) {
+ if (link->stream->ssl) {
+--
+2.4.4
+
diff --git a/SOURCES/0039-worker-validate-correctly-surfaces.patch b/SOURCES/0039-worker-validate-correctly-surfaces.patch
deleted file mode 100644
index f7a89d8..0000000
--- a/SOURCES/0039-worker-validate-correctly-surfaces.patch
+++ /dev/null
@@ -1,136 +0,0 @@
-From bea51d967731a2be7741fcbeef799b9fd925343a Mon Sep 17 00:00:00 2001
-From: Frediano Ziglio <fziglio@redhat.com>
-Date: Wed, 9 Sep 2015 12:42:09 +0100
-Subject: [PATCH 1/2] worker: validate correctly surfaces
-
-Do not just give warning and continue to use an invalid index into
-an array.
-
-Resolves: CVE-2015-5260
-
-Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
-Acked-by: Christophe Fergeau <cfergeau@redhat.com>
----
- server/red_worker.c | 33 ++++++++++++++++++---------------
- 1 file changed, 18 insertions(+), 15 deletions(-)
-
-diff --git a/server/red_worker.c b/server/red_worker.c
-index 93e3398..c62dbcb 100644
---- a/server/red_worker.c
-+++ b/server/red_worker.c
-@@ -1054,6 +1054,7 @@ typedef struct BitmapData {
- SpiceRect lossy_rect;
- } BitmapData;
-
-+static inline int validate_surface(RedWorker *worker, uint32_t surface_id);
- static void red_draw_qxl_drawable(RedWorker *worker, Drawable *drawable);
- static void red_current_flush(RedWorker *worker, int surface_id);
- #ifdef DRAW_ALL
-@@ -1267,11 +1268,6 @@ static inline int is_primary_surface(RedWorker *worker, uint32_t surface_id)
- return FALSE;
- }
-
--static inline void __validate_surface(RedWorker *worker, uint32_t surface_id)
--{
-- spice_warn_if(surface_id >= worker->n_surfaces);
--}
--
- static int validate_drawable_bbox(RedWorker *worker, RedDrawable *drawable)
- {
- DrawContext *context;
-@@ -1280,7 +1276,7 @@ static int validate_drawable_bbox(RedWorker *worker, RedDrawable *drawable)
- /* surface_id must be validated before calling into
- * validate_drawable_bbox
- */
-- __validate_surface(worker, surface_id);
-+ VALIDATE_SURFACE_RETVAL(worker, surface_id, FALSE);
- context = &worker->surfaces[surface_id].context;
-
- if (drawable->bbox.top < 0)
-@@ -1301,7 +1297,10 @@ static int validate_drawable_bbox(RedWorker *worker, RedDrawable *drawable)
-
- static inline int validate_surface(RedWorker *worker, uint32_t surface_id)
- {
-- spice_warn_if(surface_id >= worker->n_surfaces);
-+ if SPICE_UNLIKELY(surface_id >= worker->n_surfaces) {
-+ spice_warning("invalid surface_id %u", surface_id);
-+ return 0;
-+ }
- if (!worker->surfaces[surface_id].context.canvas) {
- spice_warning("canvas address is %p for %d (and is NULL)\n",
- &(worker->surfaces[surface_id].context.canvas), surface_id);
-@@ -4304,12 +4303,14 @@ static inline void red_create_surface(RedWorker *worker, uint32_t surface_id,uin
- static inline void red_process_surface(RedWorker *worker, RedSurfaceCmd *surface,
- uint32_t group_id, int loadvm)
- {
-- int surface_id;
-+ uint32_t surface_id;
- RedSurface *red_surface;
- uint8_t *data;
-
- surface_id = surface->surface_id;
-- __validate_surface(worker, surface_id);
-+ if SPICE_UNLIKELY(surface_id >= worker->n_surfaces) {
-+ goto exit;
-+ }
-
- red_surface = &worker->surfaces[surface_id];
-
-@@ -4345,6 +4346,7 @@ static inline void red_process_surface(RedWorker *worker, RedSurfaceCmd *surface
- default:
- spice_error("unknown surface command");
- };
-+exit:
- red_put_surface_cmd(surface);
- free(surface);
- }
-@@ -11091,7 +11093,7 @@ void handle_dev_update(void *opaque, void *payload)
- {
- RedWorker *worker = opaque;
- RedWorkerMessageUpdate *msg = payload;
-- SpiceRect *rect = spice_new0(SpiceRect, 1);
-+ SpiceRect *rect;
- RedSurface *surface;
- uint32_t surface_id = msg->surface_id;
- const QXLRect *qxl_area = msg->qxl_area;
-@@ -11099,17 +11101,16 @@ void handle_dev_update(void *opaque, void *payload)
- QXLRect *qxl_dirty_rects = msg->qxl_dirty_rects;
- uint32_t clear_dirty_region = msg->clear_dirty_region;
-
-+ VALIDATE_SURFACE_RET(worker, surface_id);
-+
-+ rect = spice_new0(SpiceRect, 1);
- surface = &worker->surfaces[surface_id];
- red_get_rect_ptr(rect, qxl_area);
- flush_display_commands(worker);
-
- spice_assert(worker->running);
-
-- if (validate_surface(worker, surface_id)) {
-- red_update_area(worker, rect, surface_id);
-- } else {
-- rendering_incorrect(__func__);
-- }
-+ red_update_area(worker, rect, surface_id);
- free(rect);
-
- surface_dirty_region_to_rects(surface, qxl_dirty_rects, num_dirty_rects,
-@@ -11148,6 +11149,7 @@ void handle_dev_del_memslot(void *opaque, void *payload)
- * surface_id == 0, maybe move the assert upward and merge the two functions? */
- static inline void destroy_surface_wait(RedWorker *worker, int surface_id)
- {
-+ VALIDATE_SURFACE_RET(worker, surface_id);
- if (!worker->surfaces[surface_id].context.canvas) {
- return;
- }
-@@ -11412,6 +11414,7 @@ void handle_dev_create_primary_surface(void *opaque, void *payload)
-
- static void dev_destroy_primary_surface(RedWorker *worker, uint32_t surface_id)
- {
-+ VALIDATE_SURFACE_RET(worker, surface_id);
- spice_warn_if(surface_id != 0);
-
- spice_debug(NULL);
---
-2.4.3
-
diff --git a/SOURCES/0040-Don-t-set-SpiceLinkReply-pub_key-if-client-advertise.patch b/SOURCES/0040-Don-t-set-SpiceLinkReply-pub_key-if-client-advertise.patch
new file mode 100644
index 0000000..47993b3
--- /dev/null
+++ b/SOURCES/0040-Don-t-set-SpiceLinkReply-pub_key-if-client-advertise.patch
@@ -0,0 +1,111 @@
+From 3a3ec08f25e2b53e9de256fcb3a4f951c4b1e871 Mon Sep 17 00:00:00 2001
+From: =?UTF-8?q?Fabiano=20Fid=C3=AAncio?= <fidencio@redhat.com>
+Date: Thu, 9 Jul 2015 01:04:31 +0200
+Subject: [PATCH 3/3] Don't set SpiceLinkReply::pub_key if client advertises
+ SASL cap
+
+If the client advertises the SASL cap, it means it guarantees it will be
+able to use SASL if the server supports, and that it does not need a valid
+SpiceLinkReply::pub_key field when using SASL.
+
+When the client cap is set, we thus don't need to create a RSA public key
+if SASL is enabled server side.
+
+The reason for needing client guarantees about not looking at the pub_key
+field is that its presence and size is hardcoded in the protocol, but in
+some hardened setups (using fips mode), generating a RSA 1024 bit key as
+expected is forbidden and fails. With this new capability, the server
+knows the client will be able to handle SASL if needed, and can skip
+the generation of the key altogether. This means that on the setups
+described above, SASL authentication has to be used.
+---
+ server/reds.c | 56 +++++++++++++++++++++++++++++++++++++++-----------------
+ 1 file changed, 39 insertions(+), 17 deletions(-)
+
+diff --git a/server/reds.c b/server/reds.c
+index c1edf67..9521416 100644
+--- a/server/reds.c
++++ b/server/reds.c
+@@ -1469,7 +1469,7 @@ static int reds_send_link_ack(RedLinkInfo *link)
+ RedChannel *channel;
+ RedChannelCapabilities *channel_caps;
+ BUF_MEM *bmBuf;
+- BIO *bio;
++ BIO *bio = NULL;
+ int ret = FALSE;
+
+ header.magic = SPICE_MAGIC;
+@@ -1494,24 +1494,45 @@ static int reds_send_link_ack(RedLinkInfo *link)
+ ack.num_channel_caps = channel_caps->num_caps;
+ header.size += (ack.num_common_caps + ack.num_channel_caps) * sizeof(uint32_t);
+ ack.caps_offset = sizeof(SpiceLinkReply);
++ if (!sasl_enabled
++ || !red_link_info_test_capability(link, SPICE_COMMON_CAP_AUTH_SASL)) {
++ if (!(link->tiTicketing.rsa = RSA_new())) {
++ spice_warning("RSA new failed");
++ return FALSE;
++ }
+
+- if (!(link->tiTicketing.rsa = RSA_new())) {
+- spice_warning("RSA nes failed");
+- return FALSE;
+- }
+-
+- if (!(bio = BIO_new(BIO_s_mem()))) {
+- spice_warning("BIO new failed");
+- return FALSE;
+- }
++ if (!(bio = BIO_new(BIO_s_mem()))) {
++ spice_warning("BIO new failed");
++ return FALSE;
++ }
+
+- RSA_generate_key_ex(link->tiTicketing.rsa, SPICE_TICKET_KEY_PAIR_LENGTH, link->tiTicketing.bn,
+- NULL);
+- link->tiTicketing.rsa_size = RSA_size(link->tiTicketing.rsa);
++ if (RSA_generate_key_ex(link->tiTicketing.rsa,
++ SPICE_TICKET_KEY_PAIR_LENGTH,
++ link->tiTicketing.bn,
++ NULL) != 1) {
++ spice_warning("Failed to generate %d bits RSA key: %s",
++ SPICE_TICKET_KEY_PAIR_LENGTH,
++ ERR_error_string(ERR_get_error(), NULL));
++ goto end;
++ }
++ link->tiTicketing.rsa_size = RSA_size(link->tiTicketing.rsa);
+
+- i2d_RSA_PUBKEY_bio(bio, link->tiTicketing.rsa);
+- BIO_get_mem_ptr(bio, &bmBuf);
+- memcpy(ack.pub_key, bmBuf->data, sizeof(ack.pub_key));
++ i2d_RSA_PUBKEY_bio(bio, link->tiTicketing.rsa);
++ BIO_get_mem_ptr(bio, &bmBuf);
++ memcpy(ack.pub_key, bmBuf->data, sizeof(ack.pub_key));
++ } else {
++ /* if the client sets the AUTH_SASL cap, it indicates that it
++ * supports SASL, and will use it if the server supports SASL as
++ * well. Moreover, a client setting the AUTH_SASL cap also
++ * indicates that it will not try using the RSA-related content
++ * in the SpiceLinkReply message, so we don't need to initialize
++ * it. Reason to avoid this is to fix auth in fips mode where
++ * the generation of a 1024 bit RSA key as we are trying to do
++ * will fail.
++ */
++ spice_warning("not initialising RSA key");
++ memset(ack.pub_key, '\0', sizeof(ack.pub_key));
++ }
+
+ if (!sync_write(link->stream, &header, sizeof(header)))
+ goto end;
+@@ -1525,7 +1546,8 @@ static int reds_send_link_ack(RedLinkInfo *link)
+ ret = TRUE;
+
+ end:
+- BIO_free(bio);
++ if (bio != NULL)
++ BIO_free(bio);
+ return ret;
+ }
+
+--
+2.4.4
+
diff --git a/SOURCES/0040-worker-avoid-double-free-or-double-create-of-surface.patch b/SOURCES/0040-worker-avoid-double-free-or-double-create-of-surface.patch
deleted file mode 100644
index 2ef2398..0000000
--- a/SOURCES/0040-worker-avoid-double-free-or-double-create-of-surface.patch
+++ /dev/null
@@ -1,46 +0,0 @@
-From 269e9d112639ab6c54645de217c46ef75617d780 Mon Sep 17 00:00:00 2001
-From: Frediano Ziglio <fziglio@redhat.com>
-Date: Wed, 9 Sep 2015 12:45:06 +0100
-Subject: [PATCH 2/2] worker: avoid double free or double create of surfaces
-
-A driver can overwrite surface state creating a surface with the same
-id of a previous one.
-Also can try to destroy surfaces that are not created.
-Both requests cause invalid internal states that could lead to crashes
-or memory corruptions.
-
-Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
----
- server/red_worker.c | 9 ++++++++-
- 1 file changed, 8 insertions(+), 1 deletion(-)
-
-diff --git a/server/red_worker.c b/server/red_worker.c
-index c62dbcb..a7eaab9 100644
---- a/server/red_worker.c
-+++ b/server/red_worker.c
-@@ -4320,6 +4320,10 @@ static inline void red_process_surface(RedWorker *worker, RedSurfaceCmd *surface
- int32_t stride = surface->u.surface_create.stride;
- int reloaded_surface = loadvm || (surface->flags & QXL_SURF_FLAG_KEEP_DATA);
-
-+ if (red_surface->refs) {
-+ spice_warning("avoiding creating a surface twice");
-+ break;
-+ }
- data = surface->u.surface_create.data;
- if (stride < 0) {
- data -= (int32_t)(stride * (height - 1));
-@@ -4333,7 +4337,10 @@ static inline void red_process_surface(RedWorker *worker, RedSurfaceCmd *surface
- break;
- }
- case QXL_SURFACE_CMD_DESTROY:
-- spice_warn_if(!red_surface->context.canvas);
-+ if (!red_surface->refs) {
-+ spice_warning("avoiding destroying a surface twice");
-+ break;
-+ }
- set_surface_release_info(worker, surface_id, 0, surface->release_info, group_id);
- red_handle_depends_on_target_surface(worker, surface_id);
- /* note that red_handle_depends_on_target_surface must be called before red_current_clear.
---
-2.4.3
-
diff --git a/SOURCES/0041-Define-a-constant-to-limit-data-from-guest.patch b/SOURCES/0041-Define-a-constant-to-limit-data-from-guest.patch
deleted file mode 100644
index d569778..0000000
--- a/SOURCES/0041-Define-a-constant-to-limit-data-from-guest.patch
+++ /dev/null
@@ -1,42 +0,0 @@
-From 247209c1f1c6a41d9fe0532ae17f19ae1cdcc2f7 Mon Sep 17 00:00:00 2001
-From: Frediano Ziglio <fziglio@redhat.com>
-Date: Tue, 8 Sep 2015 11:58:11 +0100
-Subject: [PATCH 41/57] Define a constant to limit data from guest.
-
-This limit will prevent guest trying to do nasty things and DoS to host.
-
-Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
----
- server/red_parse_qxl.c | 11 +++++++++++
- 1 file changed, 11 insertions(+)
-
-diff --git a/server/red_parse_qxl.c b/server/red_parse_qxl.c
-index 6c0b065..4449f2c 100644
---- a/server/red_parse_qxl.c
-+++ b/server/red_parse_qxl.c
-@@ -21,11 +21,22 @@
-
- #include <stdbool.h>
- #include <inttypes.h>
-+#include <glib.h>
- #include "common/lz_common.h"
- #include "red_common.h"
- #include "red_memslots.h"
- #include "red_parse_qxl.h"
-
-+/* Max size in bytes for any data field used in a QXL command.
-+ * This will for example be useful to prevent the guest from saturating the
-+ * host memory if it tries to send overlapping chunks.
-+ * This value should be big enough for all requests but limited
-+ * to 32 bits. Even better if it fits on 31 bits to detect integer overflows.
-+ */
-+#define MAX_DATA_CHUNK 0x7ffffffflu
-+
-+G_STATIC_ASSERT(MAX_DATA_CHUNK <= G_MAXINT32);
-+
- #if 0
- static void hexdump_qxl(RedMemSlotInfo *slots, int group_id,
- QXLPHYSICAL addr, uint8_t bytes)
---
-2.4.3
-
diff --git a/SOURCES/0041-server-don-t-assert-on-invalid-client-message.patch b/SOURCES/0041-server-don-t-assert-on-invalid-client-message.patch
new file mode 100644
index 0000000..a3750ca
--- /dev/null
+++ b/SOURCES/0041-server-don-t-assert-on-invalid-client-message.patch
@@ -0,0 +1,33 @@
+From cf19a886b8ce41ce7c5159a2898f9587318a32ad Mon Sep 17 00:00:00 2001
+From: =?UTF-8?q?Marc-Andr=C3=A9=20Lureau?= <marcandre.lureau@gmail.com>
+Date: Wed, 25 Jun 2014 14:36:03 +0200
+Subject: [PATCH] server: don't assert on invalid client message
+
+Some users have been reaching this error:
+snd_receive: ASSERT n failed
+
+A misbehaving client could easily hit that condition by sending too big
+messages. Instead of assert(), replace with a warning. When a message
+too big to fit is received, it will simply disconnect the channel.
+
+https://bugzilla.redhat.com/show_bug.cgi?id=962187
+---
+ server/snd_worker.c | 2 +-
+ 1 file changed, 1 insertion(+), 1 deletion(-)
+
+diff --git a/server/snd_worker.c b/server/snd_worker.c
+index ebddfcd..c451031 100644
+--- a/server/snd_worker.c
++++ b/server/snd_worker.c
+@@ -421,7 +421,7 @@ static void snd_receive(void* data)
+ for (;;) {
+ ssize_t n;
+ n = channel->recive_data.end - channel->recive_data.now;
+- spice_assert(n);
++ spice_warn_if(n <= 0);
+ n = reds_stream_read(channel->stream, channel->recive_data.now, n);
+ if (n <= 0) {
+ if (n == 0) {
+--
+2.4.4
+
diff --git a/SOURCES/0042-Don-t-truncate-large-now-values-in-_spice_timer_set.patch b/SOURCES/0042-Don-t-truncate-large-now-values-in-_spice_timer_set.patch
new file mode 100644
index 0000000..792da4f
--- /dev/null
+++ b/SOURCES/0042-Don-t-truncate-large-now-values-in-_spice_timer_set.patch
@@ -0,0 +1,80 @@
+From d6f73e30020e0e2cbe6ee48d5f1bf38e0587ba83 Mon Sep 17 00:00:00 2001
+From: David Gibson <dgibson@redhat.com>
+Date: Mon, 10 Mar 2014 11:55:47 +0100
+Subject: [PATCH] Don't truncate large 'now' values in _spice_timer_set
+
+static void _spice_timer_set(SpiceTimer *timer, uint32_t ms, uint32_t now)
+
+The _spice_timer_set() function takes a 32-bit integer for the "now" value.
+The now value passed in however, can exceed 2^32 (it's in ms and derived
+from CLOCK_MONOTONIC, which will wrap around a 32-bit integer in around 46
+days).
+
+If the now value passed in exceeds 2^32, this will mean timers are inserted
+into the active list with expiry values before the current time, they will
+immediately trigger, and (if they don't make themselves inactive) be
+reinserted still before the current time.
+
+This leads to an infinite loop in spice_timer_queue_cb().
+
+https://bugzilla.redhat.com/show_bug.cgi?id=1072700
+---
+ server/spice_timer_queue.c | 13 +++++++------
+ 1 file changed, 7 insertions(+), 6 deletions(-)
+
+diff --git a/server/spice_timer_queue.c b/server/spice_timer_queue.c
+index 8f6e9c8..71de84a 100644
+--- a/server/spice_timer_queue.c
++++ b/server/spice_timer_queue.c
+@@ -147,7 +147,7 @@ SpiceTimer *spice_timer_queue_add(SpiceTimerFunc func, void *opaque)
+ return timer;
+ }
+
+-static void _spice_timer_set(SpiceTimer *timer, uint32_t ms, uint32_t now)
++static void _spice_timer_set(SpiceTimer *timer, uint32_t ms, uint64_t now)
+ {
+ RingItem *next_item;
+ SpiceTimerQueue *queue;
+@@ -183,7 +183,8 @@ void spice_timer_set(SpiceTimer *timer, uint32_t ms)
+ spice_assert(pthread_equal(timer->queue->thread, pthread_self()) != 0);
+
+ clock_gettime(CLOCK_MONOTONIC, &now);
+- _spice_timer_set(timer, ms, now.tv_sec * 1000 + (now.tv_nsec / 1000 / 1000));
++ _spice_timer_set(timer, ms,
++ (uint64_t)now.tv_sec * 1000 + (now.tv_nsec / 1000 / 1000));
+ }
+
+ void spice_timer_cancel(SpiceTimer *timer)
+@@ -217,7 +218,7 @@ void spice_timer_remove(SpiceTimer *timer)
+ unsigned int spice_timer_queue_get_timeout_ms(void)
+ {
+ struct timespec now;
+- int now_ms;
++ int64_t now_ms;
+ RingItem *head;
+ SpiceTimer *head_timer;
+ SpiceTimerQueue *queue = spice_timer_queue_find_with_lock();
+@@ -232,9 +233,9 @@ unsigned int spice_timer_queue_get_timeout_ms(void)
+ head_timer = SPICE_CONTAINEROF(head, SpiceTimer, active_link);
+
+ clock_gettime(CLOCK_MONOTONIC, &now);
+- now_ms = (now.tv_sec * 1000) - (now.tv_nsec / 1000 / 1000);
++ now_ms = ((int64_t)now.tv_sec * 1000) - (now.tv_nsec / 1000 / 1000);
+
+- return MAX(0, ((int)head_timer->expiry_time - now_ms));
++ return MAX(0, ((int64_t)head_timer->expiry_time - now_ms));
+ }
+
+
+@@ -252,7 +253,7 @@ void spice_timer_queue_cb(void)
+ }
+
+ clock_gettime(CLOCK_MONOTONIC, &now);
+- now_ms = (now.tv_sec * 1000) + (now.tv_nsec / 1000 / 1000);
++ now_ms = ((uint64_t)now.tv_sec * 1000) + (now.tv_nsec / 1000 / 1000);
+
+ while ((head = ring_get_head(&queue->active_timers))) {
+ SpiceTimer *timer = SPICE_CONTAINEROF(head, SpiceTimer, active_link);
+--
+2.4.4
+
diff --git a/SOURCES/0042-Fix-some-integer-overflow-causing-large-memory-alloc.patch b/SOURCES/0042-Fix-some-integer-overflow-causing-large-memory-alloc.patch
deleted file mode 100644
index ff5e6af..0000000
--- a/SOURCES/0042-Fix-some-integer-overflow-causing-large-memory-alloc.patch
+++ /dev/null
@@ -1,69 +0,0 @@
-From b9ee3c381ef823b6a4f246e0df3112efdd349b6a Mon Sep 17 00:00:00 2001
-From: Frediano Ziglio <fziglio@redhat.com>
-Date: Thu, 17 Sep 2015 15:00:22 +0100
-Subject: [PATCH 42/57] Fix some integer overflow causing large memory
- allocations
-
-Prevent integer overflow when computing image sizes.
-Image index computations are done using 32 bit so this can cause easily
-security issues. MAX_DATA_CHUNK is larger than the virtual
-card limit, so this is not going to cause change in behaviours.
-Comparing size calculation results with MAX_DATA_CHUNK will allow us to
-catch overflows.
-Prevent guest from allocating large amount of memory.
-
-Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
----
- server/red_parse_qxl.c | 15 +++++++++++----
- 1 file changed, 11 insertions(+), 4 deletions(-)
-
-diff --git a/server/red_parse_qxl.c b/server/red_parse_qxl.c
-index 4449f2c..ceb2759 100644
---- a/server/red_parse_qxl.c
-+++ b/server/red_parse_qxl.c
-@@ -384,7 +384,7 @@ static SpiceImage *red_get_image(RedMemSlotInfo *slots, int group_id,
- QXLImage *qxl;
- SpiceImage *red = NULL;
- SpicePalette *rp = NULL;
-- size_t bitmap_size, size;
-+ uint64_t bitmap_size, size;
- uint8_t qxl_flags;
- int error;
-
-@@ -460,7 +460,10 @@ static SpiceImage *red_get_image(RedMemSlotInfo *slots, int group_id,
- red->u.bitmap.palette = rp;
- red->u.bitmap.palette_id = rp->unique;
- }
-- bitmap_size = red->u.bitmap.y * abs(red->u.bitmap.stride);
-+ bitmap_size = (uint64_t) red->u.bitmap.y * red->u.bitmap.stride;
-+ if (bitmap_size > MAX_DATA_CHUNK) {
-+ goto error;
-+ }
- if (qxl_flags & QXL_BITMAP_DIRECT) {
- red->u.bitmap.data = red_get_image_data_flat(slots, group_id,
- qxl->bitmap.data,
-@@ -1220,7 +1223,7 @@ int red_get_surface_cmd(RedMemSlotInfo *slots, int group_id,
- RedSurfaceCmd *red, QXLPHYSICAL addr)
- {
- QXLSurfaceCmd *qxl;
-- size_t size;
-+ uint64_t size;
- int error;
-
- qxl = (QXLSurfaceCmd *)get_virt(slots, addr, sizeof(*qxl), group_id,
-@@ -1240,7 +1243,11 @@ int red_get_surface_cmd(RedMemSlotInfo *slots, int group_id,
- red->u.surface_create.width = qxl->u.surface_create.width;
- red->u.surface_create.height = qxl->u.surface_create.height;
- red->u.surface_create.stride = qxl->u.surface_create.stride;
-- size = red->u.surface_create.height * abs(red->u.surface_create.stride);
-+ /* the multiplication can overflow, also abs(-2^31) may return a negative value */
-+ size = (uint64_t) red->u.surface_create.height * abs(red->u.surface_create.stride);
-+ if (size > MAX_DATA_CHUNK || red->u.surface_create.stride == G_MININT32) {
-+ return 1;
-+ }
- red->u.surface_create.data =
- (uint8_t*)get_virt(slots, qxl->u.surface_create.data, size, group_id, &error);
- if (error) {
---
-2.4.3
-
diff --git a/SOURCES/0043-Avoid-race-conditions-reading-monitor-configs-from-g.patch b/SOURCES/0043-Avoid-race-conditions-reading-monitor-configs-from-g.patch
new file mode 100644
index 0000000..c26de66
--- /dev/null
+++ b/SOURCES/0043-Avoid-race-conditions-reading-monitor-configs-from-g.patch
@@ -0,0 +1,120 @@
+From 4249d114abe6ace0553b6cfd1464e220d1e5acb1 Mon Sep 17 00:00:00 2001
+From: Frediano Ziglio <fziglio@redhat.com>
+Date: Tue, 9 Jun 2015 08:50:46 +0100
+Subject: [PATCH] Avoid race conditions reading monitor configs from guest
+
+For security reasons do not assume guest do not change structures it
+pass to Qemu.
+Guest could change count field while Qemu is copying QXLMonitorsConfig
+structure leading to heap corruption.
+This patch avoid it reading count only once.
+
+Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
+---
+ server/red_worker.c | 46 ++++++++++++++++++++++++++++++++--------------
+ 1 file changed, 32 insertions(+), 14 deletions(-)
+
+diff --git a/server/red_worker.c b/server/red_worker.c
+index 9e6a6ad..955cac2 100644
+--- a/server/red_worker.c
++++ b/server/red_worker.c
+@@ -11270,7 +11270,8 @@ static inline void red_monitors_config_item_add(DisplayChannelClient *dcc)
+ }
+
+ static void worker_update_monitors_config(RedWorker *worker,
+- QXLMonitorsConfig *dev_monitors_config)
++ QXLMonitorsConfig *dev_monitors_config,
++ uint16_t count, uint16_t max_allowed)
+ {
+ int heads_size;
+ MonitorsConfig *monitors_config;
+@@ -11279,22 +11280,22 @@ static void worker_update_monitors_config(RedWorker *worker,
+ monitors_config_decref(worker->monitors_config);
+
+ spice_debug("monitors config %d(%d)",
+- dev_monitors_config->count,
+- dev_monitors_config->max_allowed);
+- for (i = 0; i < dev_monitors_config->count; i++) {
++ count,
++ max_allowed);
++ for (i = 0; i < count; i++) {
+ spice_debug("+%d+%d %dx%d",
+ dev_monitors_config->heads[i].x,
+ dev_monitors_config->heads[i].y,
+ dev_monitors_config->heads[i].width,
+ dev_monitors_config->heads[i].height);
+ }
+- heads_size = dev_monitors_config->count * sizeof(QXLHead);
++ heads_size = count * sizeof(QXLHead);
+ worker->monitors_config = monitors_config =
+ spice_malloc(sizeof(*monitors_config) + heads_size);
+ monitors_config->refs = 1;
+ monitors_config->worker = worker;
+- monitors_config->count = dev_monitors_config->count;
+- monitors_config->max_allowed = dev_monitors_config->max_allowed;
++ monitors_config->count = count;
++ monitors_config->max_allowed = max_allowed;
+ memcpy(monitors_config->heads, dev_monitors_config->heads, heads_size);
+ }
+
+@@ -11678,33 +11679,50 @@ void handle_dev_display_migrate(void *opaque, void *payload)
+ red_migrate_display(worker, rcc);
+ }
+
++static inline uint32_t qxl_monitors_config_size(uint32_t heads)
++{
++ return sizeof(QXLMonitorsConfig) + sizeof(QXLHead) * heads;
++}
++
+ static void handle_dev_monitors_config_async(void *opaque, void *payload)
+ {
+ RedWorkerMessageMonitorsConfigAsync *msg = payload;
+ RedWorker *worker = opaque;
+- int min_size = sizeof(QXLMonitorsConfig) + sizeof(QXLHead);
+ int error;
++ uint16_t count, max_allowed;
+ QXLMonitorsConfig *dev_monitors_config =
+ (QXLMonitorsConfig*)get_virt(&worker->mem_slots, msg->monitors_config,
+- min_size, msg->group_id, &error);
++ qxl_monitors_config_size(1),
++ msg->group_id, &error);
+
+ if (error) {
+ /* TODO: raise guest bug (requires added QXL interface) */
+ return;
+ }
+ worker->driver_cap_monitors_config = 1;
+- if (dev_monitors_config->count == 0) {
++ count = dev_monitors_config->count;
++ max_allowed = dev_monitors_config->max_allowed;
++ if (count == 0) {
+ spice_warning("ignoring an empty monitors config message from driver");
+ return;
+ }
+- if (dev_monitors_config->count > dev_monitors_config->max_allowed) {
++ if (count > max_allowed) {
+ spice_warning("ignoring malformed monitors_config from driver, "
+ "count > max_allowed %d > %d",
+- dev_monitors_config->count,
+- dev_monitors_config->max_allowed);
++ count,
++ max_allowed);
++ return;
++ }
++ /* get pointer again to check virtual size */
++ dev_monitors_config =
++ (QXLMonitorsConfig*)get_virt(&worker->mem_slots, msg->monitors_config,
++ qxl_monitors_config_size(count),
++ msg->group_id, &error);
++ if (error) {
++ /* TODO: raise guest bug (requires added QXL interface) */
+ return;
+ }
+- worker_update_monitors_config(worker, dev_monitors_config);
++ worker_update_monitors_config(worker, dev_monitors_config, count, max_allowed);
+ red_worker_push_monitors_config(worker);
+ }
+
+--
+2.4.4
+
diff --git a/SOURCES/0043-Check-properly-surface-to-be-created.patch b/SOURCES/0043-Check-properly-surface-to-be-created.patch
deleted file mode 100644
index 05c24d9..0000000
--- a/SOURCES/0043-Check-properly-surface-to-be-created.patch
+++ /dev/null
@@ -1,78 +0,0 @@
-From b15d9c6d94bf426d61aaa4631ed55271c0d12b14 Mon Sep 17 00:00:00 2001
-From: Frediano Ziglio <fziglio@redhat.com>
-Date: Tue, 8 Sep 2015 16:02:59 +0100
-Subject: [PATCH 43/57] Check properly surface to be created
-
-Check format is valid.
-Check stride is at least the size of required bytes for a row.
-
-Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
-Acked-by: Christophe Fergeau <cfergeau@redhat.com>
----
- server/red_parse_qxl.c | 35 ++++++++++++++++++++++++++++++++++-
- 1 file changed, 34 insertions(+), 1 deletion(-)
-
-diff --git a/server/red_parse_qxl.c b/server/red_parse_qxl.c
-index ceb2759..a7ca71d 100644
---- a/server/red_parse_qxl.c
-+++ b/server/red_parse_qxl.c
-@@ -1219,12 +1219,30 @@ void red_put_message(RedMessage *red)
- /* nothing yet */
- }
-
-+static unsigned int surface_format_to_bpp(uint32_t format)
-+{
-+ switch (format) {
-+ case SPICE_SURFACE_FMT_1_A:
-+ return 1;
-+ case SPICE_SURFACE_FMT_8_A:
-+ return 8;
-+ case SPICE_SURFACE_FMT_16_555:
-+ case SPICE_SURFACE_FMT_16_565:
-+ return 16;
-+ case SPICE_SURFACE_FMT_32_xRGB:
-+ case SPICE_SURFACE_FMT_32_ARGB:
-+ return 32;
-+ }
-+ return 0;
-+}
-+
- int red_get_surface_cmd(RedMemSlotInfo *slots, int group_id,
- RedSurfaceCmd *red, QXLPHYSICAL addr)
- {
- QXLSurfaceCmd *qxl;
- uint64_t size;
- int error;
-+ unsigned int bpp;
-
- qxl = (QXLSurfaceCmd *)get_virt(slots, addr, sizeof(*qxl), group_id,
- &error);
-@@ -1243,9 +1261,24 @@ int red_get_surface_cmd(RedMemSlotInfo *slots, int group_id,
- red->u.surface_create.width = qxl->u.surface_create.width;
- red->u.surface_create.height = qxl->u.surface_create.height;
- red->u.surface_create.stride = qxl->u.surface_create.stride;
-+ bpp = surface_format_to_bpp(red->u.surface_create.format);
-+
-+ /* check if format is valid */
-+ if (!bpp) {
-+ return 1;
-+ }
-+
-+ /* check stride is larger than required bytes */
-+ size = ((uint64_t) red->u.surface_create.width * bpp + 7u) / 8u;
-+ /* the uint32_t conversion is here to avoid problems with -2^31 value */
-+ if (red->u.surface_create.stride == G_MININT32
-+ || size > (uint32_t) abs(red->u.surface_create.stride)) {
-+ return 1;
-+ }
-+
- /* the multiplication can overflow, also abs(-2^31) may return a negative value */
- size = (uint64_t) red->u.surface_create.height * abs(red->u.surface_create.stride);
-- if (size > MAX_DATA_CHUNK || red->u.surface_create.stride == G_MININT32) {
-+ if (size > MAX_DATA_CHUNK) {
- return 1;
- }
- red->u.surface_create.data =
---
-2.4.3
-
diff --git a/SOURCES/0044-Fix-buffer-reading-overflow.patch b/SOURCES/0044-Fix-buffer-reading-overflow.patch
deleted file mode 100644
index bc0240d..0000000
--- a/SOURCES/0044-Fix-buffer-reading-overflow.patch
+++ /dev/null
@@ -1,38 +0,0 @@
-From 18087073df84885642d9b0b1efd0e86e18409bbe Mon Sep 17 00:00:00 2001
-From: Frediano Ziglio <fziglio@redhat.com>
-Date: Tue, 8 Sep 2015 10:00:37 +0100
-Subject: [PATCH 44/57] Fix buffer reading overflow
-
-Not security risk as just for read.
-However, this could be used to attempt integer overflows in the
-following lines.
-
-Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
-Acked-by: Christophe Fergeau <cfergeau@redhat.com>
----
- server/red_parse_qxl.c | 9 ++++++++-
- 1 file changed, 8 insertions(+), 1 deletion(-)
-
-diff --git a/server/red_parse_qxl.c b/server/red_parse_qxl.c
-index a7ca71d..01cba0f 100644
---- a/server/red_parse_qxl.c
-+++ b/server/red_parse_qxl.c
-@@ -361,7 +361,14 @@ static const int MAP_BITMAP_FMT_TO_BITS_PER_PIXEL[] = {0, 1, 1, 4, 4, 8, 16, 24,
-
- static int bitmap_consistent(SpiceBitmap *bitmap)
- {
-- int bpp = MAP_BITMAP_FMT_TO_BITS_PER_PIXEL[bitmap->format];
-+ int bpp;
-+
-+ if (bitmap->format >= SPICE_N_ELEMENTS(MAP_BITMAP_FMT_TO_BITS_PER_PIXEL)) {
-+ spice_warning("wrong format specified for image\n");
-+ return FALSE;
-+ }
-+
-+ bpp = MAP_BITMAP_FMT_TO_BITS_PER_PIXEL[bitmap->format];
-
- if (bitmap->stride < ((bitmap->x * bpp + 7) / 8)) {
- spice_error("image stride too small for width: %d < ((%d * %d + 7) / 8) (%s=%d)\n",
---
-2.4.3
-
diff --git a/SOURCES/0044-Lock-the-pixmap-image-cache-for-the-entire-fill_bits.patch b/SOURCES/0044-Lock-the-pixmap-image-cache-for-the-entire-fill_bits.patch
new file mode 100644
index 0000000..bcc9e80
--- /dev/null
+++ b/SOURCES/0044-Lock-the-pixmap-image-cache-for-the-entire-fill_bits.patch
@@ -0,0 +1,223 @@
+From 6f7e2fca1030e38f3a82f09b30a4839923f17c66 Mon Sep 17 00:00:00 2001
+From: Sandy Stutsman <sstutsma@redhat.com>
+Date: Fri, 26 Jun 2015 11:59:13 -0400
+Subject: [PATCH] Lock the pixmap image cache for the entire fill_bits call
+
+Locking the individual calls that access the pixmap cache in fill_bits is
+not adequately thread safe. Often a windows guest with multiple monitors
+will be sending the same image via different threads. Both threads can
+be in fill_bits at the same time making changes to the cache for the same
+image. This can result in images being deleted before all the client
+channels are finished with them or with the same image being send multiple
+times. Here's what can happen with out the lock in fill_bits
+
+On the server in red_worker.c:fill_bits
+ Thread 1 calls pixmap_cache_hit for Image A and finds it isn't in cache
+ Thread 2 calls pixmap_cache_hit for Image A and finds it isn't in cache
+
+ Thread 1 adds Image 1 to pixmap_cache (1x)
+ Thread 2 adds Image 1 to pixmap_cache (2x)
+
+On the client
+ Channel 1 adds Image A to image_cache (1x)
+ Channel 2 replaces Image A in image_cache (1x)
+
+On server
+ Thread 1 sends Image A rendering commands
+ Thread N removes Image A from pixmap_cache (image remains - 1x)
+ Thread 2 sends Image A rendering commands
+
+On client
+ Channe1 renders from Image A
+ Channel N removes Image a from image_cache (image is completely removed)
+ Channel2 render command hangs waiting for Image A
+---
+ server/red_client_shared_cache.h | 24 ++++++++++++------------
+ server/red_worker.c | 23 +++++++++++++++--------
+ 2 files changed, 27 insertions(+), 20 deletions(-)
+
+diff --git a/server/red_client_shared_cache.h b/server/red_client_shared_cache.h
+index 821ee18..7feb28e 100644
+--- a/server/red_client_shared_cache.h
++++ b/server/red_client_shared_cache.h
+@@ -36,13 +36,12 @@
+
+ #define CHANNEL_FROM_RCC(rcc) SPICE_CONTAINEROF((rcc)->channel, CHANNEL, common.base);
+
+-static int FUNC_NAME(hit)(CACHE *cache, uint64_t id, int *lossy, DisplayChannelClient *dcc)
++static int FUNC_NAME(unlocked_hit)(CACHE *cache, uint64_t id, int *lossy, DisplayChannelClient *dcc)
+ {
+ NewCacheItem *item;
+ uint64_t serial;
+
+ serial = red_channel_client_get_message_serial(&dcc->common.base);
+- pthread_mutex_lock(&cache->lock);
+ item = cache->hash_table[CACHE_HASH_KEY(id)];
+
+ while (item) {
+@@ -57,15 +56,22 @@ static int FUNC_NAME(hit)(CACHE *cache, uint64_t id, int *lossy, DisplayChannelC
+ }
+ item = item->next;
+ }
+- pthread_mutex_unlock(&cache->lock);
+
+ return !!item;
+ }
+
+-static int FUNC_NAME(set_lossy)(CACHE *cache, uint64_t id, int lossy)
++static int FUNC_NAME(hit)(CACHE *cache, uint64_t id, int *lossy, DisplayChannelClient *dcc)
+ {
+- NewCacheItem *item;
++ int hit;
+ pthread_mutex_lock(&cache->lock);
++ hit = FUNC_NAME(unlocked_hit)(cache,id,lossy, dcc);
++ pthread_mutex_unlock(&cache->lock);
++ return hit;
++}
++
++static int FUNC_NAME(unlocked_set_lossy)(CACHE *cache, uint64_t id, int lossy)
++{
++ NewCacheItem *item;
+
+ item = cache->hash_table[CACHE_HASH_KEY(id)];
+
+@@ -76,11 +82,10 @@ static int FUNC_NAME(set_lossy)(CACHE *cache, uint64_t id, int lossy)
+ }
+ item = item->next;
+ }
+- pthread_mutex_unlock(&cache->lock);
+ return !!item;
+ }
+
+-static int FUNC_NAME(add)(CACHE *cache, uint64_t id, uint32_t size, int lossy, DisplayChannelClient *dcc)
++static int FUNC_NAME(unlocked_add)(CACHE *cache, uint64_t id, uint32_t size, int lossy, DisplayChannelClient *dcc)
+ {
+ NewCacheItem *item;
+ uint64_t serial;
+@@ -91,15 +96,12 @@ static int FUNC_NAME(add)(CACHE *cache, uint64_t id, uint32_t size, int lossy, D
+ item = spice_new(NewCacheItem, 1);
+ serial = red_channel_client_get_message_serial(&dcc->common.base);
+
+- pthread_mutex_lock(&cache->lock);
+-
+ if (cache->generation != dcc->CACH_GENERATION) {
+ if (!dcc->pending_pixmaps_sync) {
+ red_channel_client_pipe_add_type(
+ &dcc->common.base, PIPE_ITEM_TYPE_PIXMAP_SYNC);
+ dcc->pending_pixmaps_sync = TRUE;
+ }
+- pthread_mutex_unlock(&cache->lock);
+ free(item);
+ return FALSE;
+ }
+@@ -112,7 +114,6 @@ static int FUNC_NAME(add)(CACHE *cache, uint64_t id, uint32_t size, int lossy, D
+ if (!(tail = (NewCacheItem *)ring_get_tail(&cache->lru)) ||
+ tail->sync[dcc->common.id] == serial) {
+ cache->available += size;
+- pthread_mutex_unlock(&cache->lock);
+ free(item);
+ return FALSE;
+ }
+@@ -144,7 +145,6 @@ static int FUNC_NAME(add)(CACHE *cache, uint64_t id, uint32_t size, int lossy, D
+ memset(item->sync, 0, sizeof(item->sync));
+ item->sync[dcc->common.id] = serial;
+ cache->sync[dcc->common.id] = serial;
+- pthread_mutex_unlock(&cache->lock);
+ return TRUE;
+ }
+
+diff --git a/server/red_worker.c b/server/red_worker.c
+index 955cac2..93e3398 100644
+--- a/server/red_worker.c
++++ b/server/red_worker.c
+@@ -6750,9 +6750,9 @@ static inline void red_display_add_image_to_pixmap_cache(RedChannelClient *rcc,
+ if ((image->descriptor.flags & SPICE_IMAGE_FLAGS_CACHE_ME)) {
+ spice_assert(image->descriptor.width * image->descriptor.height > 0);
+ if (!(io_image->descriptor.flags & SPICE_IMAGE_FLAGS_CACHE_REPLACE_ME)) {
+- if (pixmap_cache_add(dcc->pixmap_cache, image->descriptor.id,
+- image->descriptor.width * image->descriptor.height, is_lossy,
+- dcc)) {
++ if (pixmap_cache_unlocked_add(dcc->pixmap_cache, image->descriptor.id,
++ image->descriptor.width * image->descriptor.height, is_lossy,
++ dcc)) {
+ io_image->descriptor.flags |= SPICE_IMAGE_FLAGS_CACHE_ME;
+ dcc->send_data.pixmap_cache_items[dcc->send_data.num_pixmap_cache_items++] =
+ image->descriptor.id;
+@@ -6797,11 +6797,12 @@ static FillBitsType fill_bits(DisplayChannelClient *dcc, SpiceMarshaller *m,
+ if (simage->descriptor.flags & SPICE_IMAGE_FLAGS_HIGH_BITS_SET) {
+ image.descriptor.flags = SPICE_IMAGE_FLAGS_HIGH_BITS_SET;
+ }
++ pthread_mutex_lock(&dcc->pixmap_cache->lock);
+
+ if ((simage->descriptor.flags & SPICE_IMAGE_FLAGS_CACHE_ME)) {
+ int lossy_cache_item;
+- if (pixmap_cache_hit(dcc->pixmap_cache, image.descriptor.id,
+- &lossy_cache_item, dcc)) {
++ if (pixmap_cache_unlocked_hit(dcc->pixmap_cache, image.descriptor.id,
++ &lossy_cache_item, dcc)) {
+ dcc->send_data.pixmap_cache_items[dcc->send_data.num_pixmap_cache_items++] =
+ image.descriptor.id;
+ if (can_lossy || !lossy_cache_item) {
+@@ -6818,10 +6819,11 @@ static FillBitsType fill_bits(DisplayChannelClient *dcc, SpiceMarshaller *m,
+ spice_assert(bitmap_palette_out == NULL);
+ spice_assert(lzplt_palette_out == NULL);
+ stat_inc_counter(display_channel->cache_hits_counter, 1);
++ pthread_mutex_unlock(&dcc->pixmap_cache->lock);
+ return FILL_BITS_TYPE_CACHE;
+ } else {
+- pixmap_cache_set_lossy(dcc->pixmap_cache, simage->descriptor.id,
+- FALSE);
++ pixmap_cache_unlocked_set_lossy(dcc->pixmap_cache, simage->descriptor.id,
++ FALSE);
+ image.descriptor.flags |= SPICE_IMAGE_FLAGS_CACHE_REPLACE_ME;
+ }
+ }
+@@ -6835,6 +6837,7 @@ static FillBitsType fill_bits(DisplayChannelClient *dcc, SpiceMarshaller *m,
+ surface_id = simage->u.surface.surface_id;
+ if (!validate_surface(worker, surface_id)) {
+ rendering_incorrect("SPICE_IMAGE_TYPE_SURFACE");
++ pthread_mutex_unlock(&dcc->pixmap_cache->lock);
+ return FILL_BITS_TYPE_SURFACE;
+ }
+
+@@ -6849,6 +6852,7 @@ static FillBitsType fill_bits(DisplayChannelClient *dcc, SpiceMarshaller *m,
+ &bitmap_palette_out, &lzplt_palette_out);
+ spice_assert(bitmap_palette_out == NULL);
+ spice_assert(lzplt_palette_out == NULL);
++ pthread_mutex_unlock(&dcc->pixmap_cache->lock);
+ return FILL_BITS_TYPE_SURFACE;
+ }
+ case SPICE_IMAGE_TYPE_BITMAP: {
+@@ -6879,6 +6883,7 @@ static FillBitsType fill_bits(DisplayChannelClient *dcc, SpiceMarshaller *m,
+ }
+
+ spice_marshaller_add_ref_chunks(m, bitmap->data);
++ pthread_mutex_unlock(&dcc->pixmap_cache->lock);
+ return FILL_BITS_TYPE_BITMAP;
+ } else {
+ red_display_add_image_to_pixmap_cache(rcc, simage, &image,
+@@ -6896,6 +6901,7 @@ static FillBitsType fill_bits(DisplayChannelClient *dcc, SpiceMarshaller *m,
+ }
+
+ spice_assert(!comp_send_data.is_lossy || can_lossy);
++ pthread_mutex_unlock(&dcc->pixmap_cache->lock);
+ return (comp_send_data.is_lossy ? FILL_BITS_TYPE_COMPRESS_LOSSY :
+ FILL_BITS_TYPE_COMPRESS_LOSSLESS);
+ }
+@@ -6909,11 +6915,12 @@ static FillBitsType fill_bits(DisplayChannelClient *dcc, SpiceMarshaller *m,
+ spice_assert(bitmap_palette_out == NULL);
+ spice_assert(lzplt_palette_out == NULL);
+ spice_marshaller_add_ref_chunks(m, image.u.quic.data);
++ pthread_mutex_unlock(&dcc->pixmap_cache->lock);
+ return FILL_BITS_TYPE_COMPRESS_LOSSLESS;
+ default:
+ spice_error("invalid image type %u", image.descriptor.type);
+ }
+-
++ pthread_mutex_unlock(&dcc->pixmap_cache->lock);
+ return 0;
+ }
+
+--
+2.4.4
+
diff --git a/SOURCES/0045-Prevent-32-bit-integer-overflow-in-bitmap_consistent.patch b/SOURCES/0045-Prevent-32-bit-integer-overflow-in-bitmap_consistent.patch
deleted file mode 100644
index 822ab6d..0000000
--- a/SOURCES/0045-Prevent-32-bit-integer-overflow-in-bitmap_consistent.patch
+++ /dev/null
@@ -1,48 +0,0 @@
-From 7baa8c39757b46a834e20198e4b18e9f1752e20e Mon Sep 17 00:00:00 2001
-From: Frediano Ziglio <fziglio@redhat.com>
-Date: Tue, 8 Sep 2015 13:09:35 +0100
-Subject: [PATCH 45/57] Prevent 32 bit integer overflow in bitmap_consistent
-
-The overflow may lead to buffer overflow as the row size computed from
-width (bitmap->x) can be bigger than the size in bytes (bitmap->stride).
-This can make spice-server accept the invalid sizes.
-
-Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
-Acked-by: Christophe Fergeau <cfergeau@redhat.com>
----
- server/red_parse_qxl.c | 9 +++++----
- 1 file changed, 5 insertions(+), 4 deletions(-)
-
-diff --git a/server/red_parse_qxl.c b/server/red_parse_qxl.c
-index 01cba0f..3385f52 100644
---- a/server/red_parse_qxl.c
-+++ b/server/red_parse_qxl.c
-@@ -357,11 +357,12 @@ static const char *bitmap_format_to_string(int format)
- return "unknown";
- }
-
--static const int MAP_BITMAP_FMT_TO_BITS_PER_PIXEL[] = {0, 1, 1, 4, 4, 8, 16, 24, 32, 32, 8};
-+static const unsigned int MAP_BITMAP_FMT_TO_BITS_PER_PIXEL[] =
-+ {0, 1, 1, 4, 4, 8, 16, 24, 32, 32, 8};
-
- static int bitmap_consistent(SpiceBitmap *bitmap)
- {
-- int bpp;
-+ unsigned int bpp;
-
- if (bitmap->format >= SPICE_N_ELEMENTS(MAP_BITMAP_FMT_TO_BITS_PER_PIXEL)) {
- spice_warning("wrong format specified for image\n");
-@@ -370,8 +371,8 @@ static int bitmap_consistent(SpiceBitmap *bitmap)
-
- bpp = MAP_BITMAP_FMT_TO_BITS_PER_PIXEL[bitmap->format];
-
-- if (bitmap->stride < ((bitmap->x * bpp + 7) / 8)) {
-- spice_error("image stride too small for width: %d < ((%d * %d + 7) / 8) (%s=%d)\n",
-+ if (bitmap->stride < (((uint64_t) bitmap->x * bpp + 7u) / 8u)) {
-+ spice_warning("image stride too small for width: %d < ((%d * %d + 7) / 8) (%s=%d)\n",
- bitmap->stride, bitmap->x, bpp,
- bitmap_format_to_string(bitmap->format),
- bitmap->format);
---
-2.4.3
-
diff --git a/SOURCES/0045-reds-Assure-we-don-t-have-stale-statistic-files-befo.patch b/SOURCES/0045-reds-Assure-we-don-t-have-stale-statistic-files-befo.patch
new file mode 100644
index 0000000..951a92e
--- /dev/null
+++ b/SOURCES/0045-reds-Assure-we-don-t-have-stale-statistic-files-befo.patch
@@ -0,0 +1,37 @@
+From 40537f6a3e3389b8377b0ae790c62ea0da8aa6d8 Mon Sep 17 00:00:00 2001
+From: Frediano Ziglio <fziglio@redhat.com>
+Date: Wed, 15 Jul 2015 14:15:52 +0100
+Subject: [PATCH] reds: Assure we don't have stale statistic files before
+ trying to create a new one
+
+If a previous Qemu executable is not able to delete the statistic file
+on the next creation with same name (statitics file are based on pid
+numbers so if pid get reused for another Qemu process you get the same
+name) it fails as you can't open a file with 0444 permissions (these
+are the permission used to create these files).
+This patch assure there are no stale file trying to remove it before the
+creation of the new one. As file is based on pid and name used for spice
+you are not deleting another file.
+
+Fixes: rhbz#1177326
+
+Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
+---
+ server/reds.c | 1 +
+ 1 file changed, 1 insertion(+)
+
+diff --git a/server/reds.c b/server/reds.c
+index 57ef07a..c74894a 100644
+--- a/server/reds.c
++++ b/server/reds.c
+@@ -3291,6 +3291,7 @@ static int do_spice_init(SpiceCoreInterface *core_interface)
+ shm_name_len = strlen(SPICE_STAT_SHM_NAME) + 20;
+ reds->stat_shm_name = (char *)spice_malloc(shm_name_len);
+ snprintf(reds->stat_shm_name, shm_name_len, SPICE_STAT_SHM_NAME, getpid());
++ shm_unlink(reds->stat_shm_name);
+ if ((fd = shm_open(reds->stat_shm_name, O_CREAT | O_RDWR, 0444)) == -1) {
+ spice_error("statistics shm_open failed, %s", strerror(errno));
+ }
+--
+2.1.0
+
diff --git a/SOURCES/0046-Fix-race-condition-on-red_get_clip_rects.patch b/SOURCES/0046-Fix-race-condition-on-red_get_clip_rects.patch
deleted file mode 100644
index 279aeca..0000000
--- a/SOURCES/0046-Fix-race-condition-on-red_get_clip_rects.patch
+++ /dev/null
@@ -1,42 +0,0 @@
-From 078a903d55f44aedd22b4fa8dd86e4b03b82c01c Mon Sep 17 00:00:00 2001
-From: Frediano Ziglio <fziglio@redhat.com>
-Date: Tue, 8 Sep 2015 10:01:51 +0100
-Subject: [PATCH 46/57] Fix race condition on red_get_clip_rects
-
-Do not read multiple time an array size that can be changed.
-
-Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
-Acked-by: Christophe Fergeau <cfergeau@redhat.com>
----
- server/red_parse_qxl.c | 8 +++++---
- 1 file changed, 5 insertions(+), 3 deletions(-)
-
-diff --git a/server/red_parse_qxl.c b/server/red_parse_qxl.c
-index 3385f52..affd3a2 100644
---- a/server/red_parse_qxl.c
-+++ b/server/red_parse_qxl.c
-@@ -273,6 +273,7 @@ static SpiceClipRects *red_get_clip_rects(RedMemSlotInfo *slots, int group_id,
- size_t size;
- int i;
- int error;
-+ uint32_t num_rects;
-
- qxl = (QXLClipRects *)get_virt(slots, addr, sizeof(*qxl), group_id, &error);
- if (error) {
-@@ -284,9 +285,10 @@ static SpiceClipRects *red_get_clip_rects(RedMemSlotInfo *slots, int group_id,
- data = red_linearize_chunk(&chunks, size, &free_data);
- red_put_data_chunks(&chunks);
-
-- spice_assert(qxl->num_rects * sizeof(QXLRect) == size);
-- red = spice_malloc(sizeof(*red) + qxl->num_rects * sizeof(SpiceRect));
-- red->num_rects = qxl->num_rects;
-+ num_rects = qxl->num_rects;
-+ spice_assert(num_rects * sizeof(QXLRect) == size);
-+ red = spice_malloc(sizeof(*red) + num_rects * sizeof(SpiceRect));
-+ red->num_rects = num_rects;
-
- start = (QXLRect*)data;
- for (i = 0; i < red->num_rects; i++) {
---
-2.4.3
-
diff --git a/SOURCES/0046-worker-validate-correctly-surfaces.patch b/SOURCES/0046-worker-validate-correctly-surfaces.patch
new file mode 100644
index 0000000..f7a89d8
--- /dev/null
+++ b/SOURCES/0046-worker-validate-correctly-surfaces.patch
@@ -0,0 +1,136 @@
+From bea51d967731a2be7741fcbeef799b9fd925343a Mon Sep 17 00:00:00 2001
+From: Frediano Ziglio <fziglio@redhat.com>
+Date: Wed, 9 Sep 2015 12:42:09 +0100
+Subject: [PATCH 1/2] worker: validate correctly surfaces
+
+Do not just give warning and continue to use an invalid index into
+an array.
+
+Resolves: CVE-2015-5260
+
+Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
+Acked-by: Christophe Fergeau <cfergeau@redhat.com>
+---
+ server/red_worker.c | 33 ++++++++++++++++++---------------
+ 1 file changed, 18 insertions(+), 15 deletions(-)
+
+diff --git a/server/red_worker.c b/server/red_worker.c
+index 93e3398..c62dbcb 100644
+--- a/server/red_worker.c
++++ b/server/red_worker.c
+@@ -1054,6 +1054,7 @@ typedef struct BitmapData {
+ SpiceRect lossy_rect;
+ } BitmapData;
+
++static inline int validate_surface(RedWorker *worker, uint32_t surface_id);
+ static void red_draw_qxl_drawable(RedWorker *worker, Drawable *drawable);
+ static void red_current_flush(RedWorker *worker, int surface_id);
+ #ifdef DRAW_ALL
+@@ -1267,11 +1268,6 @@ static inline int is_primary_surface(RedWorker *worker, uint32_t surface_id)
+ return FALSE;
+ }
+
+-static inline void __validate_surface(RedWorker *worker, uint32_t surface_id)
+-{
+- spice_warn_if(surface_id >= worker->n_surfaces);
+-}
+-
+ static int validate_drawable_bbox(RedWorker *worker, RedDrawable *drawable)
+ {
+ DrawContext *context;
+@@ -1280,7 +1276,7 @@ static int validate_drawable_bbox(RedWorker *worker, RedDrawable *drawable)
+ /* surface_id must be validated before calling into
+ * validate_drawable_bbox
+ */
+- __validate_surface(worker, surface_id);
++ VALIDATE_SURFACE_RETVAL(worker, surface_id, FALSE);
+ context = &worker->surfaces[surface_id].context;
+
+ if (drawable->bbox.top < 0)
+@@ -1301,7 +1297,10 @@ static int validate_drawable_bbox(RedWorker *worker, RedDrawable *drawable)
+
+ static inline int validate_surface(RedWorker *worker, uint32_t surface_id)
+ {
+- spice_warn_if(surface_id >= worker->n_surfaces);
++ if SPICE_UNLIKELY(surface_id >= worker->n_surfaces) {
++ spice_warning("invalid surface_id %u", surface_id);
++ return 0;
++ }
+ if (!worker->surfaces[surface_id].context.canvas) {
+ spice_warning("canvas address is %p for %d (and is NULL)\n",
+ &(worker->surfaces[surface_id].context.canvas), surface_id);
+@@ -4304,12 +4303,14 @@ static inline void red_create_surface(RedWorker *worker, uint32_t surface_id,uin
+ static inline void red_process_surface(RedWorker *worker, RedSurfaceCmd *surface,
+ uint32_t group_id, int loadvm)
+ {
+- int surface_id;
++ uint32_t surface_id;
+ RedSurface *red_surface;
+ uint8_t *data;
+
+ surface_id = surface->surface_id;
+- __validate_surface(worker, surface_id);
++ if SPICE_UNLIKELY(surface_id >= worker->n_surfaces) {
++ goto exit;
++ }
+
+ red_surface = &worker->surfaces[surface_id];
+
+@@ -4345,6 +4346,7 @@ static inline void red_process_surface(RedWorker *worker, RedSurfaceCmd *surface
+ default:
+ spice_error("unknown surface command");
+ };
++exit:
+ red_put_surface_cmd(surface);
+ free(surface);
+ }
+@@ -11091,7 +11093,7 @@ void handle_dev_update(void *opaque, void *payload)
+ {
+ RedWorker *worker = opaque;
+ RedWorkerMessageUpdate *msg = payload;
+- SpiceRect *rect = spice_new0(SpiceRect, 1);
++ SpiceRect *rect;
+ RedSurface *surface;
+ uint32_t surface_id = msg->surface_id;
+ const QXLRect *qxl_area = msg->qxl_area;
+@@ -11099,17 +11101,16 @@ void handle_dev_update(void *opaque, void *payload)
+ QXLRect *qxl_dirty_rects = msg->qxl_dirty_rects;
+ uint32_t clear_dirty_region = msg->clear_dirty_region;
+
++ VALIDATE_SURFACE_RET(worker, surface_id);
++
++ rect = spice_new0(SpiceRect, 1);
+ surface = &worker->surfaces[surface_id];
+ red_get_rect_ptr(rect, qxl_area);
+ flush_display_commands(worker);
+
+ spice_assert(worker->running);
+
+- if (validate_surface(worker, surface_id)) {
+- red_update_area(worker, rect, surface_id);
+- } else {
+- rendering_incorrect(__func__);
+- }
++ red_update_area(worker, rect, surface_id);
+ free(rect);
+
+ surface_dirty_region_to_rects(surface, qxl_dirty_rects, num_dirty_rects,
+@@ -11148,6 +11149,7 @@ void handle_dev_del_memslot(void *opaque, void *payload)
+ * surface_id == 0, maybe move the assert upward and merge the two functions? */
+ static inline void destroy_surface_wait(RedWorker *worker, int surface_id)
+ {
++ VALIDATE_SURFACE_RET(worker, surface_id);
+ if (!worker->surfaces[surface_id].context.canvas) {
+ return;
+ }
+@@ -11412,6 +11414,7 @@ void handle_dev_create_primary_surface(void *opaque, void *payload)
+
+ static void dev_destroy_primary_surface(RedWorker *worker, uint32_t surface_id)
+ {
++ VALIDATE_SURFACE_RET(worker, surface_id);
+ spice_warn_if(surface_id != 0);
+
+ spice_debug(NULL);
+--
+2.4.3
+
diff --git a/SOURCES/0047-Fix-race-in-red_get_image.patch b/SOURCES/0047-Fix-race-in-red_get_image.patch
deleted file mode 100644
index abaf5b3..0000000
--- a/SOURCES/0047-Fix-race-in-red_get_image.patch
+++ /dev/null
@@ -1,77 +0,0 @@
-From d2fc5cee16c10e53d81c6251e6929da54270a6f4 Mon Sep 17 00:00:00 2001
-From: Frediano Ziglio <fziglio@redhat.com>
-Date: Tue, 8 Sep 2015 10:04:10 +0100
-Subject: [PATCH 47/57] Fix race in red_get_image
-
-Do not read multiple times data from guest as this could be changed
-by other vcpu threads.
-This causes races and security problems if these data are used for
-buffer allocation or checks.
-
-Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
-Acked-by: Christophe Fergeau <cfergeau@redhat.com>
----
- server/red_parse_qxl.c | 18 ++++++++++--------
- 1 file changed, 10 insertions(+), 8 deletions(-)
-
-diff --git a/server/red_parse_qxl.c b/server/red_parse_qxl.c
-index affd3a2..84ea526 100644
---- a/server/red_parse_qxl.c
-+++ b/server/red_parse_qxl.c
-@@ -397,6 +397,7 @@ static SpiceImage *red_get_image(RedMemSlotInfo *slots, int group_id,
- uint64_t bitmap_size, size;
- uint8_t qxl_flags;
- int error;
-+ QXLPHYSICAL palette;
-
- if (addr == 0) {
- return NULL;
-@@ -422,12 +423,16 @@ static SpiceImage *red_get_image(RedMemSlotInfo *slots, int group_id,
- switch (red->descriptor.type) {
- case SPICE_IMAGE_TYPE_BITMAP:
- red->u.bitmap.format = qxl->bitmap.format;
-- if (!bitmap_fmt_is_rgb(qxl->bitmap.format) && !qxl->bitmap.palette && !is_mask) {
-+ red->u.bitmap.x = qxl->bitmap.x;
-+ red->u.bitmap.y = qxl->bitmap.y;
-+ red->u.bitmap.stride = qxl->bitmap.stride;
-+ palette = qxl->bitmap.palette;
-+ if (!bitmap_fmt_is_rgb(red->u.bitmap.format) && !palette && !is_mask) {
- spice_warning("guest error: missing palette on bitmap format=%d\n",
- red->u.bitmap.format);
- goto error;
- }
-- if (qxl->bitmap.x == 0 || qxl->bitmap.y == 0) {
-+ if (red->u.bitmap.x == 0 || red->u.bitmap.y == 0) {
- spice_warning("guest error: zero area bitmap\n");
- goto error;
- }
-@@ -435,23 +440,20 @@ static SpiceImage *red_get_image(RedMemSlotInfo *slots, int group_id,
- if (qxl_flags & QXL_BITMAP_TOP_DOWN) {
- red->u.bitmap.flags = SPICE_BITMAP_FLAGS_TOP_DOWN;
- }
-- red->u.bitmap.x = qxl->bitmap.x;
-- red->u.bitmap.y = qxl->bitmap.y;
-- red->u.bitmap.stride = qxl->bitmap.stride;
- if (!bitmap_consistent(&red->u.bitmap)) {
- goto error;
- }
-- if (qxl->bitmap.palette) {
-+ if (palette) {
- QXLPalette *qp;
- int i, num_ents;
-- qp = (QXLPalette *)get_virt(slots, qxl->bitmap.palette,
-+ qp = (QXLPalette *)get_virt(slots, palette,
- sizeof(*qp), group_id, &error);
- if (error) {
- goto error;
- }
- num_ents = qp->num_ents;
- if (!validate_virt(slots, (intptr_t)qp->ents,
-- get_memslot_id(slots, qxl->bitmap.palette),
-+ get_memslot_id(slots, palette),
- num_ents * sizeof(qp->ents[0]), group_id)) {
- goto error;
- }
---
-2.4.3
-
diff --git a/SOURCES/0047-worker-avoid-double-free-or-double-create-of-surface.patch b/SOURCES/0047-worker-avoid-double-free-or-double-create-of-surface.patch
new file mode 100644
index 0000000..2ef2398
--- /dev/null
+++ b/SOURCES/0047-worker-avoid-double-free-or-double-create-of-surface.patch
@@ -0,0 +1,46 @@
+From 269e9d112639ab6c54645de217c46ef75617d780 Mon Sep 17 00:00:00 2001
+From: Frediano Ziglio <fziglio@redhat.com>
+Date: Wed, 9 Sep 2015 12:45:06 +0100
+Subject: [PATCH 2/2] worker: avoid double free or double create of surfaces
+
+A driver can overwrite surface state creating a surface with the same
+id of a previous one.
+Also can try to destroy surfaces that are not created.
+Both requests cause invalid internal states that could lead to crashes
+or memory corruptions.
+
+Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
+---
+ server/red_worker.c | 9 ++++++++-
+ 1 file changed, 8 insertions(+), 1 deletion(-)
+
+diff --git a/server/red_worker.c b/server/red_worker.c
+index c62dbcb..a7eaab9 100644
+--- a/server/red_worker.c
++++ b/server/red_worker.c
+@@ -4320,6 +4320,10 @@ static inline void red_process_surface(RedWorker *worker, RedSurfaceCmd *surface
+ int32_t stride = surface->u.surface_create.stride;
+ int reloaded_surface = loadvm || (surface->flags & QXL_SURF_FLAG_KEEP_DATA);
+
++ if (red_surface->refs) {
++ spice_warning("avoiding creating a surface twice");
++ break;
++ }
+ data = surface->u.surface_create.data;
+ if (stride < 0) {
+ data -= (int32_t)(stride * (height - 1));
+@@ -4333,7 +4337,10 @@ static inline void red_process_surface(RedWorker *worker, RedSurfaceCmd *surface
+ break;
+ }
+ case QXL_SURFACE_CMD_DESTROY:
+- spice_warn_if(!red_surface->context.canvas);
++ if (!red_surface->refs) {
++ spice_warning("avoiding destroying a surface twice");
++ break;
++ }
+ set_surface_release_info(worker, surface_id, 0, surface->release_info, group_id);
+ red_handle_depends_on_target_surface(worker, surface_id);
+ /* note that red_handle_depends_on_target_surface must be called before red_current_clear.
+--
+2.4.3
+
diff --git a/SOURCES/0048-Define-a-constant-to-limit-data-from-guest.patch b/SOURCES/0048-Define-a-constant-to-limit-data-from-guest.patch
new file mode 100644
index 0000000..d306102
--- /dev/null
+++ b/SOURCES/0048-Define-a-constant-to-limit-data-from-guest.patch
@@ -0,0 +1,42 @@
+From 2e1cac5508cd04815e0e624cdbc436857934f689 Mon Sep 17 00:00:00 2001
+From: Frediano Ziglio <fziglio@redhat.com>
+Date: Tue, 8 Sep 2015 11:58:11 +0100
+Subject: [PATCH 48/64] Define a constant to limit data from guest.
+
+This limit will prevent guest trying to do nasty things and DoS to host.
+
+Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
+---
+ server/red_parse_qxl.c | 11 +++++++++++
+ 1 file changed, 11 insertions(+)
+
+diff --git a/server/red_parse_qxl.c b/server/red_parse_qxl.c
+index 6c0b065..4449f2c 100644
+--- a/server/red_parse_qxl.c
++++ b/server/red_parse_qxl.c
+@@ -21,11 +21,22 @@
+
+ #include <stdbool.h>
+ #include <inttypes.h>
++#include <glib.h>
+ #include "common/lz_common.h"
+ #include "red_common.h"
+ #include "red_memslots.h"
+ #include "red_parse_qxl.h"
+
++/* Max size in bytes for any data field used in a QXL command.
++ * This will for example be useful to prevent the guest from saturating the
++ * host memory if it tries to send overlapping chunks.
++ * This value should be big enough for all requests but limited
++ * to 32 bits. Even better if it fits on 31 bits to detect integer overflows.
++ */
++#define MAX_DATA_CHUNK 0x7ffffffflu
++
++G_STATIC_ASSERT(MAX_DATA_CHUNK <= G_MAXINT32);
++
+ #if 0
+ static void hexdump_qxl(RedMemSlotInfo *slots, int group_id,
+ QXLPHYSICAL addr, uint8_t bytes)
+--
+2.4.3
+
diff --git a/SOURCES/0048-Fix-race-condition-in-red_get_string.patch b/SOURCES/0048-Fix-race-condition-in-red_get_string.patch
deleted file mode 100644
index 8bc6bb5..0000000
--- a/SOURCES/0048-Fix-race-condition-in-red_get_string.patch
+++ /dev/null
@@ -1,62 +0,0 @@
-From 932e27e50032c1c7032be3616217a2ab0586fe78 Mon Sep 17 00:00:00 2001
-From: Frediano Ziglio <fziglio@redhat.com>
-Date: Tue, 8 Sep 2015 10:05:20 +0100
-Subject: [PATCH 48/57] Fix race condition in red_get_string
-
-Do not read multiple time an array size that can be changed.
-
-Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
-Acked-by: Christophe Fergeau <cfergeau@redhat.com>
----
- server/red_parse_qxl.c | 15 +++++++++------
- 1 file changed, 9 insertions(+), 6 deletions(-)
-
-diff --git a/server/red_parse_qxl.c b/server/red_parse_qxl.c
-index 84ea526..2d4636e 100644
---- a/server/red_parse_qxl.c
-+++ b/server/red_parse_qxl.c
-@@ -809,6 +809,7 @@ static SpiceString *red_get_string(RedMemSlotInfo *slots, int group_id,
- size_t chunk_size, qxl_size, red_size, glyph_size;
- int glyphs, bpp = 0, i;
- int error;
-+ uint16_t qxl_flags, qxl_length;
-
- qxl = (QXLString *)get_virt(slots, addr, sizeof(*qxl), group_id, &error);
- if (error) {
-@@ -825,13 +826,15 @@ static SpiceString *red_get_string(RedMemSlotInfo *slots, int group_id,
- red_put_data_chunks(&chunks);
-
- qxl_size = qxl->data_size;
-+ qxl_flags = qxl->flags;
-+ qxl_length = qxl->length;
- spice_assert(chunk_size == qxl_size);
-
-- if (qxl->flags & SPICE_STRING_FLAGS_RASTER_A1) {
-+ if (qxl_flags & SPICE_STRING_FLAGS_RASTER_A1) {
- bpp = 1;
-- } else if (qxl->flags & SPICE_STRING_FLAGS_RASTER_A4) {
-+ } else if (qxl_flags & SPICE_STRING_FLAGS_RASTER_A4) {
- bpp = 4;
-- } else if (qxl->flags & SPICE_STRING_FLAGS_RASTER_A8) {
-+ } else if (qxl_flags & SPICE_STRING_FLAGS_RASTER_A8) {
- bpp = 8;
- }
- spice_assert(bpp != 0);
-@@ -848,11 +851,11 @@ static SpiceString *red_get_string(RedMemSlotInfo *slots, int group_id,
- start = (QXLRasterGlyph*)(&start->data[glyph_size]);
- }
- spice_assert(start <= end);
-- spice_assert(glyphs == qxl->length);
-+ spice_assert(glyphs == qxl_length);
-
- red = spice_malloc(red_size);
-- red->length = qxl->length;
-- red->flags = qxl->flags;
-+ red->length = qxl_length;
-+ red->flags = qxl_flags;
-
- start = (QXLRasterGlyph*)data;
- end = (QXLRasterGlyph*)(data + chunk_size);
---
-2.4.3
-
diff --git a/SOURCES/0049-Fix-integer-overflow-computing-glyph_size-in-red_get.patch b/SOURCES/0049-Fix-integer-overflow-computing-glyph_size-in-red_get.patch
deleted file mode 100644
index 9bf8b82..0000000
--- a/SOURCES/0049-Fix-integer-overflow-computing-glyph_size-in-red_get.patch
+++ /dev/null
@@ -1,63 +0,0 @@
-From e28c08d63490a2fb6b8cc07bf968eb16243e9c63 Mon Sep 17 00:00:00 2001
-From: Frediano Ziglio <fziglio@redhat.com>
-Date: Tue, 8 Sep 2015 10:13:24 +0100
-Subject: [PATCH 49/57] Fix integer overflow computing glyph_size in
- red_get_string
-
-If bpp is int the formula can lead to weird overflows. width and height
-are uint16_t so the formula is:
-
- size_t = u16 * (u16 * int + const_int) / const_int;
-
-so it became
-
- size_t = (int) u16 * ((int) u16 * int + const_int) / const_int;
-
-However the (int) u16 * (int) u16 can then became negative to overflow.
-Under 64 bit architectures size_t is 64 and int usually 32 so converting
-this negative 32 bit number to a unsigned 64 bit lead to a very big
-number as the signed is extended and then converted to unsigned.
-Using unsigned arithmetic prevent extending the sign.
-
-Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
-Acked-by: Christophe Fergeau <cfergeau@redhat.com>
----
- server/red_parse_qxl.c | 8 +++++---
- 1 file changed, 5 insertions(+), 3 deletions(-)
-
-diff --git a/server/red_parse_qxl.c b/server/red_parse_qxl.c
-index 2d4636e..c4b82be 100644
---- a/server/red_parse_qxl.c
-+++ b/server/red_parse_qxl.c
-@@ -807,7 +807,9 @@ static SpiceString *red_get_string(RedMemSlotInfo *slots, int group_id,
- uint8_t *data;
- bool free_data;
- size_t chunk_size, qxl_size, red_size, glyph_size;
-- int glyphs, bpp = 0, i;
-+ int glyphs, i;
-+ /* use unsigned to prevent integer overflow in multiplication below */
-+ unsigned int bpp = 0;
- int error;
- uint16_t qxl_flags, qxl_length;
-
-@@ -846,7 +848,7 @@ static SpiceString *red_get_string(RedMemSlotInfo *slots, int group_id,
- while (start < end) {
- spice_assert((QXLRasterGlyph*)(&start->data[0]) <= end);
- glyphs++;
-- glyph_size = start->height * ((start->width * bpp + 7) / 8);
-+ glyph_size = start->height * ((start->width * bpp + 7u) / 8u);
- red_size += sizeof(SpiceRasterGlyph *) + SPICE_ALIGN(sizeof(SpiceRasterGlyph) + glyph_size, 4);
- start = (QXLRasterGlyph*)(&start->data[glyph_size]);
- }
-@@ -867,7 +869,7 @@ static SpiceString *red_get_string(RedMemSlotInfo *slots, int group_id,
- glyph->height = start->height;
- red_get_point_ptr(&glyph->render_pos, &start->render_pos);
- red_get_point_ptr(&glyph->glyph_origin, &start->glyph_origin);
-- glyph_size = glyph->height * ((glyph->width * bpp + 7) / 8);
-+ glyph_size = glyph->height * ((glyph->width * bpp + 7u) / 8u);
- spice_assert((QXLRasterGlyph*)(&start->data[glyph_size]) <= end);
- memcpy(glyph->data, start->data, glyph_size);
- start = (QXLRasterGlyph*)(&start->data[glyph_size]);
---
-2.4.3
-
diff --git a/SOURCES/0049-Fix-some-integer-overflow-causing-large-memory-alloc.patch b/SOURCES/0049-Fix-some-integer-overflow-causing-large-memory-alloc.patch
new file mode 100644
index 0000000..6b24d9e
--- /dev/null
+++ b/SOURCES/0049-Fix-some-integer-overflow-causing-large-memory-alloc.patch
@@ -0,0 +1,69 @@
+From c0860a1bce777a2b54a31a01d09a71e80657b5ff Mon Sep 17 00:00:00 2001
+From: Frediano Ziglio <fziglio@redhat.com>
+Date: Thu, 17 Sep 2015 15:00:22 +0100
+Subject: [PATCH 49/64] Fix some integer overflow causing large memory
+ allocations
+
+Prevent integer overflow when computing image sizes.
+Image index computations are done using 32 bit so this can cause easily
+security issues. MAX_DATA_CHUNK is larger than the virtual
+card limit, so this is not going to cause change in behaviours.
+Comparing size calculation results with MAX_DATA_CHUNK will allow us to
+catch overflows.
+Prevent guest from allocating large amount of memory.
+
+Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
+---
+ server/red_parse_qxl.c | 15 +++++++++++----
+ 1 file changed, 11 insertions(+), 4 deletions(-)
+
+diff --git a/server/red_parse_qxl.c b/server/red_parse_qxl.c
+index 4449f2c..ceb2759 100644
+--- a/server/red_parse_qxl.c
++++ b/server/red_parse_qxl.c
+@@ -384,7 +384,7 @@ static SpiceImage *red_get_image(RedMemSlotInfo *slots, int group_id,
+ QXLImage *qxl;
+ SpiceImage *red = NULL;
+ SpicePalette *rp = NULL;
+- size_t bitmap_size, size;
++ uint64_t bitmap_size, size;
+ uint8_t qxl_flags;
+ int error;
+
+@@ -460,7 +460,10 @@ static SpiceImage *red_get_image(RedMemSlotInfo *slots, int group_id,
+ red->u.bitmap.palette = rp;
+ red->u.bitmap.palette_id = rp->unique;
+ }
+- bitmap_size = red->u.bitmap.y * abs(red->u.bitmap.stride);
++ bitmap_size = (uint64_t) red->u.bitmap.y * red->u.bitmap.stride;
++ if (bitmap_size > MAX_DATA_CHUNK) {
++ goto error;
++ }
+ if (qxl_flags & QXL_BITMAP_DIRECT) {
+ red->u.bitmap.data = red_get_image_data_flat(slots, group_id,
+ qxl->bitmap.data,
+@@ -1220,7 +1223,7 @@ int red_get_surface_cmd(RedMemSlotInfo *slots, int group_id,
+ RedSurfaceCmd *red, QXLPHYSICAL addr)
+ {
+ QXLSurfaceCmd *qxl;
+- size_t size;
++ uint64_t size;
+ int error;
+
+ qxl = (QXLSurfaceCmd *)get_virt(slots, addr, sizeof(*qxl), group_id,
+@@ -1240,7 +1243,11 @@ int red_get_surface_cmd(RedMemSlotInfo *slots, int group_id,
+ red->u.surface_create.width = qxl->u.surface_create.width;
+ red->u.surface_create.height = qxl->u.surface_create.height;
+ red->u.surface_create.stride = qxl->u.surface_create.stride;
+- size = red->u.surface_create.height * abs(red->u.surface_create.stride);
++ /* the multiplication can overflow, also abs(-2^31) may return a negative value */
++ size = (uint64_t) red->u.surface_create.height * abs(red->u.surface_create.stride);
++ if (size > MAX_DATA_CHUNK || red->u.surface_create.stride == G_MININT32) {
++ return 1;
++ }
+ red->u.surface_create.data =
+ (uint8_t*)get_virt(slots, qxl->u.surface_create.data, size, group_id, &error);
+ if (error) {
+--
+2.4.3
+
diff --git a/SOURCES/0050-Check-properly-surface-to-be-created.patch b/SOURCES/0050-Check-properly-surface-to-be-created.patch
new file mode 100644
index 0000000..f40ce03
--- /dev/null
+++ b/SOURCES/0050-Check-properly-surface-to-be-created.patch
@@ -0,0 +1,78 @@
+From b541f856f5c7fd70c1dad688306bcdfadfc2e4b9 Mon Sep 17 00:00:00 2001
+From: Frediano Ziglio <fziglio@redhat.com>
+Date: Tue, 8 Sep 2015 16:02:59 +0100
+Subject: [PATCH 50/64] Check properly surface to be created
+
+Check format is valid.
+Check stride is at least the size of required bytes for a row.
+
+Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
+Acked-by: Christophe Fergeau <cfergeau@redhat.com>
+---
+ server/red_parse_qxl.c | 35 ++++++++++++++++++++++++++++++++++-
+ 1 file changed, 34 insertions(+), 1 deletion(-)
+
+diff --git a/server/red_parse_qxl.c b/server/red_parse_qxl.c
+index ceb2759..a7ca71d 100644
+--- a/server/red_parse_qxl.c
++++ b/server/red_parse_qxl.c
+@@ -1219,12 +1219,30 @@ void red_put_message(RedMessage *red)
+ /* nothing yet */
+ }
+
++static unsigned int surface_format_to_bpp(uint32_t format)
++{
++ switch (format) {
++ case SPICE_SURFACE_FMT_1_A:
++ return 1;
++ case SPICE_SURFACE_FMT_8_A:
++ return 8;
++ case SPICE_SURFACE_FMT_16_555:
++ case SPICE_SURFACE_FMT_16_565:
++ return 16;
++ case SPICE_SURFACE_FMT_32_xRGB:
++ case SPICE_SURFACE_FMT_32_ARGB:
++ return 32;
++ }
++ return 0;
++}
++
+ int red_get_surface_cmd(RedMemSlotInfo *slots, int group_id,
+ RedSurfaceCmd *red, QXLPHYSICAL addr)
+ {
+ QXLSurfaceCmd *qxl;
+ uint64_t size;
+ int error;
++ unsigned int bpp;
+
+ qxl = (QXLSurfaceCmd *)get_virt(slots, addr, sizeof(*qxl), group_id,
+ &error);
+@@ -1243,9 +1261,24 @@ int red_get_surface_cmd(RedMemSlotInfo *slots, int group_id,
+ red->u.surface_create.width = qxl->u.surface_create.width;
+ red->u.surface_create.height = qxl->u.surface_create.height;
+ red->u.surface_create.stride = qxl->u.surface_create.stride;
++ bpp = surface_format_to_bpp(red->u.surface_create.format);
++
++ /* check if format is valid */
++ if (!bpp) {
++ return 1;
++ }
++
++ /* check stride is larger than required bytes */
++ size = ((uint64_t) red->u.surface_create.width * bpp + 7u) / 8u;
++ /* the uint32_t conversion is here to avoid problems with -2^31 value */
++ if (red->u.surface_create.stride == G_MININT32
++ || size > (uint32_t) abs(red->u.surface_create.stride)) {
++ return 1;
++ }
++
+ /* the multiplication can overflow, also abs(-2^31) may return a negative value */
+ size = (uint64_t) red->u.surface_create.height * abs(red->u.surface_create.stride);
+- if (size > MAX_DATA_CHUNK || red->u.surface_create.stride == G_MININT32) {
++ if (size > MAX_DATA_CHUNK) {
+ return 1;
+ }
+ red->u.surface_create.data =
+--
+2.4.3
+
diff --git a/SOURCES/0050-Fix-race-condition-in-red_get_data_chunks_ptr.patch b/SOURCES/0050-Fix-race-condition-in-red_get_data_chunks_ptr.patch
deleted file mode 100644
index 71b0a38..0000000
--- a/SOURCES/0050-Fix-race-condition-in-red_get_data_chunks_ptr.patch
+++ /dev/null
@@ -1,66 +0,0 @@
-From 20979131dee0f81972ca159e5f754d42890e75e6 Mon Sep 17 00:00:00 2001
-From: Frediano Ziglio <fziglio@redhat.com>
-Date: Tue, 8 Sep 2015 12:12:19 +0100
-Subject: [PATCH 50/57] Fix race condition in red_get_data_chunks_ptr
-
-Do not read multiple times data from guest as this can be changed by
-other guest vcpus. This causes races and security problems if these
-data are used for buffer allocation or checks.
-
-Actually, the 'data' member can't change during read as it is just a
-pointer to a fixed array contained in qxl. However, this change will
-make it clear that there can be no race condition.
-
-Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
----
- server/red_parse_qxl.c | 17 ++++++++++-------
- 1 file changed, 10 insertions(+), 7 deletions(-)
-
-diff --git a/server/red_parse_qxl.c b/server/red_parse_qxl.c
-index c4b82be..7cc20e6 100644
---- a/server/red_parse_qxl.c
-+++ b/server/red_parse_qxl.c
-@@ -102,30 +102,33 @@ static size_t red_get_data_chunks_ptr(RedMemSlotInfo *slots, int group_id,
- RedDataChunk *red_prev;
- size_t data_size = 0;
- int error;
-+ QXLPHYSICAL next_chunk;
-
- red->data_size = qxl->data_size;
- data_size += red->data_size;
-- if (!validate_virt(slots, (intptr_t)qxl->data, memslot_id, red->data_size, group_id)) {
-+ red->data = qxl->data;
-+ if (!validate_virt(slots, (intptr_t)red->data, memslot_id, red->data_size, group_id)) {
-+ red->data = NULL;
- return 0;
- }
-- red->data = qxl->data;
- red->prev_chunk = NULL;
-
-- while (qxl->next_chunk) {
-+ while ((next_chunk = qxl->next_chunk) != 0) {
- red_prev = red;
- red = spice_new(RedDataChunk, 1);
-- memslot_id = get_memslot_id(slots, qxl->next_chunk);
-- qxl = (QXLDataChunk *)get_virt(slots, qxl->next_chunk, sizeof(*qxl), group_id,
-+ memslot_id = get_memslot_id(slots, next_chunk);
-+ qxl = (QXLDataChunk *)get_virt(slots, next_chunk, sizeof(*qxl), group_id,
- &error);
- if (error) {
- return 0;
- }
- red->data_size = qxl->data_size;
- data_size += red->data_size;
-- if (!validate_virt(slots, (intptr_t)qxl->data, memslot_id, red->data_size, group_id)) {
-+ red->data = qxl->data;
-+ if (!validate_virt(slots, (intptr_t)red->data, memslot_id, red->data_size, group_id)) {
-+ red->data = NULL;
- return 0;
- }
-- red->data = qxl->data;
- red->prev_chunk = red_prev;
- red_prev->next_chunk = red;
- }
---
-2.4.3
-
diff --git a/SOURCES/0051-Fix-buffer-reading-overflow.patch b/SOURCES/0051-Fix-buffer-reading-overflow.patch
new file mode 100644
index 0000000..db3b272
--- /dev/null
+++ b/SOURCES/0051-Fix-buffer-reading-overflow.patch
@@ -0,0 +1,38 @@
+From 214a0c88ecbac0229ddc2d7067a8a3ce9821df5c Mon Sep 17 00:00:00 2001
+From: Frediano Ziglio <fziglio@redhat.com>
+Date: Tue, 8 Sep 2015 10:00:37 +0100
+Subject: [PATCH 51/64] Fix buffer reading overflow
+
+Not security risk as just for read.
+However, this could be used to attempt integer overflows in the
+following lines.
+
+Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
+Acked-by: Christophe Fergeau <cfergeau@redhat.com>
+---
+ server/red_parse_qxl.c | 9 ++++++++-
+ 1 file changed, 8 insertions(+), 1 deletion(-)
+
+diff --git a/server/red_parse_qxl.c b/server/red_parse_qxl.c
+index a7ca71d..01cba0f 100644
+--- a/server/red_parse_qxl.c
++++ b/server/red_parse_qxl.c
+@@ -361,7 +361,14 @@ static const int MAP_BITMAP_FMT_TO_BITS_PER_PIXEL[] = {0, 1, 1, 4, 4, 8, 16, 24,
+
+ static int bitmap_consistent(SpiceBitmap *bitmap)
+ {
+- int bpp = MAP_BITMAP_FMT_TO_BITS_PER_PIXEL[bitmap->format];
++ int bpp;
++
++ if (bitmap->format >= SPICE_N_ELEMENTS(MAP_BITMAP_FMT_TO_BITS_PER_PIXEL)) {
++ spice_warning("wrong format specified for image\n");
++ return FALSE;
++ }
++
++ bpp = MAP_BITMAP_FMT_TO_BITS_PER_PIXEL[bitmap->format];
+
+ if (bitmap->stride < ((bitmap->x * bpp + 7) / 8)) {
+ spice_error("image stride too small for width: %d < ((%d * %d + 7) / 8) (%s=%d)\n",
+--
+2.4.3
+
diff --git a/SOURCES/0051-Prevent-memory-leak-if-red_get_data_chunks_ptr-fails.patch b/SOURCES/0051-Prevent-memory-leak-if-red_get_data_chunks_ptr-fails.patch
deleted file mode 100644
index 331c3a2..0000000
--- a/SOURCES/0051-Prevent-memory-leak-if-red_get_data_chunks_ptr-fails.patch
+++ /dev/null
@@ -1,75 +0,0 @@
-From 173645bea63ad31ba0d6d4c58a06272aa488fb92 Mon Sep 17 00:00:00 2001
-From: Frediano Ziglio <fziglio@redhat.com>
-Date: Tue, 8 Sep 2015 12:14:55 +0100
-Subject: [PATCH 51/57] Prevent memory leak if red_get_data_chunks_ptr fails
-
-Free linked list if client tries to do nasty things
-
-Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
-Acked-by: Christophe Fergeau <cfergeau@redhat.com>
----
- server/red_parse_qxl.c | 31 ++++++++++++++++++++-----------
- 1 file changed, 20 insertions(+), 11 deletions(-)
-
-diff --git a/server/red_parse_qxl.c b/server/red_parse_qxl.c
-index 7cc20e6..fe3ae78 100644
---- a/server/red_parse_qxl.c
-+++ b/server/red_parse_qxl.c
-@@ -107,34 +107,43 @@ static size_t red_get_data_chunks_ptr(RedMemSlotInfo *slots, int group_id,
- red->data_size = qxl->data_size;
- data_size += red->data_size;
- red->data = qxl->data;
-+ red->prev_chunk = red->next_chunk = NULL;
- if (!validate_virt(slots, (intptr_t)red->data, memslot_id, red->data_size, group_id)) {
- red->data = NULL;
- return 0;
- }
-- red->prev_chunk = NULL;
-
- while ((next_chunk = qxl->next_chunk) != 0) {
- red_prev = red;
-- red = spice_new(RedDataChunk, 1);
-+ red = spice_new0(RedDataChunk, 1);
-+ red->prev_chunk = red_prev;
-+ red_prev->next_chunk = red;
-+
- memslot_id = get_memslot_id(slots, next_chunk);
- qxl = (QXLDataChunk *)get_virt(slots, next_chunk, sizeof(*qxl), group_id,
- &error);
-- if (error) {
-- return 0;
-- }
-+ if (error)
-+ goto error;
- red->data_size = qxl->data_size;
- data_size += red->data_size;
- red->data = qxl->data;
-- if (!validate_virt(slots, (intptr_t)red->data, memslot_id, red->data_size, group_id)) {
-- red->data = NULL;
-- return 0;
-- }
-- red->prev_chunk = red_prev;
-- red_prev->next_chunk = red;
-+ if (!validate_virt(slots, (intptr_t)red->data, memslot_id, red->data_size, group_id))
-+ goto error;
- }
-
- red->next_chunk = NULL;
- return data_size;
-+
-+error:
-+ while (red->prev_chunk) {
-+ red_prev = red->prev_chunk;
-+ free(red);
-+ red = red_prev;
-+ }
-+ red->data_size = 0;
-+ red->next_chunk = NULL;
-+ red->data = NULL;
-+ return 0;
- }
-
- static size_t red_get_data_chunks(RedMemSlotInfo *slots, int group_id,
---
-2.4.3
-
diff --git a/SOURCES/0052-Prevent-32-bit-integer-overflow-in-bitmap_consistent.patch b/SOURCES/0052-Prevent-32-bit-integer-overflow-in-bitmap_consistent.patch
new file mode 100644
index 0000000..e075eb4
--- /dev/null
+++ b/SOURCES/0052-Prevent-32-bit-integer-overflow-in-bitmap_consistent.patch
@@ -0,0 +1,48 @@
+From 0ede6619b828335a10ba94fe2739e01d5162d81c Mon Sep 17 00:00:00 2001
+From: Frediano Ziglio <fziglio@redhat.com>
+Date: Tue, 8 Sep 2015 13:09:35 +0100
+Subject: [PATCH 52/64] Prevent 32 bit integer overflow in bitmap_consistent
+
+The overflow may lead to buffer overflow as the row size computed from
+width (bitmap->x) can be bigger than the size in bytes (bitmap->stride).
+This can make spice-server accept the invalid sizes.
+
+Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
+Acked-by: Christophe Fergeau <cfergeau@redhat.com>
+---
+ server/red_parse_qxl.c | 9 +++++----
+ 1 file changed, 5 insertions(+), 4 deletions(-)
+
+diff --git a/server/red_parse_qxl.c b/server/red_parse_qxl.c
+index 01cba0f..3385f52 100644
+--- a/server/red_parse_qxl.c
++++ b/server/red_parse_qxl.c
+@@ -357,11 +357,12 @@ static const char *bitmap_format_to_string(int format)
+ return "unknown";
+ }
+
+-static const int MAP_BITMAP_FMT_TO_BITS_PER_PIXEL[] = {0, 1, 1, 4, 4, 8, 16, 24, 32, 32, 8};
++static const unsigned int MAP_BITMAP_FMT_TO_BITS_PER_PIXEL[] =
++ {0, 1, 1, 4, 4, 8, 16, 24, 32, 32, 8};
+
+ static int bitmap_consistent(SpiceBitmap *bitmap)
+ {
+- int bpp;
++ unsigned int bpp;
+
+ if (bitmap->format >= SPICE_N_ELEMENTS(MAP_BITMAP_FMT_TO_BITS_PER_PIXEL)) {
+ spice_warning("wrong format specified for image\n");
+@@ -370,8 +371,8 @@ static int bitmap_consistent(SpiceBitmap *bitmap)
+
+ bpp = MAP_BITMAP_FMT_TO_BITS_PER_PIXEL[bitmap->format];
+
+- if (bitmap->stride < ((bitmap->x * bpp + 7) / 8)) {
+- spice_error("image stride too small for width: %d < ((%d * %d + 7) / 8) (%s=%d)\n",
++ if (bitmap->stride < (((uint64_t) bitmap->x * bpp + 7u) / 8u)) {
++ spice_warning("image stride too small for width: %d < ((%d * %d + 7) / 8) (%s=%d)\n",
+ bitmap->stride, bitmap->x, bpp,
+ bitmap_format_to_string(bitmap->format),
+ bitmap->format);
+--
+2.4.3
+
diff --git a/SOURCES/0052-Prevent-DoS-from-guest-trying-to-allocate-too-much-d.patch b/SOURCES/0052-Prevent-DoS-from-guest-trying-to-allocate-too-much-d.patch
deleted file mode 100644
index eb26991..0000000
--- a/SOURCES/0052-Prevent-DoS-from-guest-trying-to-allocate-too-much-d.patch
+++ /dev/null
@@ -1,102 +0,0 @@
-From ffbe1b2e3bff5fec022030836a9319d57c6f94c5 Mon Sep 17 00:00:00 2001
-From: Frediano Ziglio <fziglio@redhat.com>
-Date: Tue, 8 Sep 2015 12:28:54 +0100
-Subject: [PATCH 52/57] Prevent DoS from guest trying to allocate too much data
- on host for chunks
-
-Limit number of chunks to a given amount to avoid guest trying to
-allocate too much memory. Using circular or nested chunks lists
-guest could try to allocate huge amounts of memory.
-Considering the list can be infinite and guest can change data this
-also prevents strange security attacks from guest.
-
-Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
----
- server/red_parse_qxl.c | 49 +++++++++++++++++++++++++++++++++++++++++--------
- 1 file changed, 41 insertions(+), 8 deletions(-)
-
-diff --git a/server/red_parse_qxl.c b/server/red_parse_qxl.c
-index fe3ae78..f183248 100644
---- a/server/red_parse_qxl.c
-+++ b/server/red_parse_qxl.c
-@@ -37,6 +37,13 @@
-
- G_STATIC_ASSERT(MAX_DATA_CHUNK <= G_MAXINT32);
-
-+/* Limit number of chunks.
-+ * The guest can attempt to make host allocate too much memory
-+ * just with a large number of small chunks.
-+ * Prevent that the chunk list take more memory than the data itself.
-+ */
-+#define MAX_CHUNKS (MAX_DATA_CHUNK/1024u)
-+
- #if 0
- static void hexdump_qxl(RedMemSlotInfo *slots, int group_id,
- QXLPHYSICAL addr, uint8_t bytes)
-@@ -100,9 +107,11 @@ static size_t red_get_data_chunks_ptr(RedMemSlotInfo *slots, int group_id,
- RedDataChunk *red, QXLDataChunk *qxl)
- {
- RedDataChunk *red_prev;
-- size_t data_size = 0;
-+ uint64_t data_size = 0;
-+ uint32_t chunk_data_size;
- int error;
- QXLPHYSICAL next_chunk;
-+ unsigned num_chunks = 0;
-
- red->data_size = qxl->data_size;
- data_size += red->data_size;
-@@ -114,19 +123,43 @@ static size_t red_get_data_chunks_ptr(RedMemSlotInfo *slots, int group_id,
- }
-
- while ((next_chunk = qxl->next_chunk) != 0) {
-+ /* somebody is trying to use too much memory using a lot of chunks.
-+ * Or made a circular list of chunks
-+ */
-+ if (++num_chunks >= MAX_CHUNKS) {
-+ spice_warning("data split in too many chunks, avoiding DoS\n");
-+ goto error;
-+ }
-+
-+ memslot_id = get_memslot_id(slots, next_chunk);
-+ qxl = (QXLDataChunk *)get_virt(slots, next_chunk, sizeof(*qxl),
-+ group_id, &error);
-+ if (error)
-+ goto error;
-+
-+ /* do not waste space for empty chunks.
-+ * This could be just a driver issue or an attempt
-+ * to allocate too much memory or a circular list.
-+ * All above cases are handled by the check for number
-+ * of chunks.
-+ */
-+ chunk_data_size = qxl->data_size;
-+ if (chunk_data_size == 0)
-+ continue;
-+
- red_prev = red;
- red = spice_new0(RedDataChunk, 1);
-+ red->data_size = chunk_data_size;
- red->prev_chunk = red_prev;
-+ red->data = qxl->data;
- red_prev->next_chunk = red;
-
-- memslot_id = get_memslot_id(slots, next_chunk);
-- qxl = (QXLDataChunk *)get_virt(slots, next_chunk, sizeof(*qxl), group_id,
-- &error);
-- if (error)
-+ data_size += chunk_data_size;
-+ /* this can happen if client is sending nested chunks */
-+ if (data_size > MAX_DATA_CHUNK) {
-+ spice_warning("too much data inside chunks, avoiding DoS\n");
- goto error;
-- red->data_size = qxl->data_size;
-- data_size += red->data_size;
-- red->data = qxl->data;
-+ }
- if (!validate_virt(slots, (intptr_t)red->data, memslot_id, red->data_size, group_id))
- goto error;
- }
---
-2.4.3
-
diff --git a/SOURCES/0053-Fix-race-condition-on-red_get_clip_rects.patch b/SOURCES/0053-Fix-race-condition-on-red_get_clip_rects.patch
new file mode 100644
index 0000000..f358959
--- /dev/null
+++ b/SOURCES/0053-Fix-race-condition-on-red_get_clip_rects.patch
@@ -0,0 +1,42 @@
+From eede1d8307b48016274077eea4dd9c7a296bf84a Mon Sep 17 00:00:00 2001
+From: Frediano Ziglio <fziglio@redhat.com>
+Date: Tue, 8 Sep 2015 10:01:51 +0100
+Subject: [PATCH 53/64] Fix race condition on red_get_clip_rects
+
+Do not read multiple time an array size that can be changed.
+
+Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
+Acked-by: Christophe Fergeau <cfergeau@redhat.com>
+---
+ server/red_parse_qxl.c | 8 +++++---
+ 1 file changed, 5 insertions(+), 3 deletions(-)
+
+diff --git a/server/red_parse_qxl.c b/server/red_parse_qxl.c
+index 3385f52..affd3a2 100644
+--- a/server/red_parse_qxl.c
++++ b/server/red_parse_qxl.c
+@@ -273,6 +273,7 @@ static SpiceClipRects *red_get_clip_rects(RedMemSlotInfo *slots, int group_id,
+ size_t size;
+ int i;
+ int error;
++ uint32_t num_rects;
+
+ qxl = (QXLClipRects *)get_virt(slots, addr, sizeof(*qxl), group_id, &error);
+ if (error) {
+@@ -284,9 +285,10 @@ static SpiceClipRects *red_get_clip_rects(RedMemSlotInfo *slots, int group_id,
+ data = red_linearize_chunk(&chunks, size, &free_data);
+ red_put_data_chunks(&chunks);
+
+- spice_assert(qxl->num_rects * sizeof(QXLRect) == size);
+- red = spice_malloc(sizeof(*red) + qxl->num_rects * sizeof(SpiceRect));
+- red->num_rects = qxl->num_rects;
++ num_rects = qxl->num_rects;
++ spice_assert(num_rects * sizeof(QXLRect) == size);
++ red = spice_malloc(sizeof(*red) + num_rects * sizeof(SpiceRect));
++ red->num_rects = num_rects;
+
+ start = (QXLRect*)data;
+ for (i = 0; i < red->num_rects; i++) {
+--
+2.4.3
+
diff --git a/SOURCES/0053-Fix-some-possible-overflows-in-red_get_string-for-32.patch b/SOURCES/0053-Fix-some-possible-overflows-in-red_get_string-for-32.patch
deleted file mode 100644
index 6875045..0000000
--- a/SOURCES/0053-Fix-some-possible-overflows-in-red_get_string-for-32.patch
+++ /dev/null
@@ -1,41 +0,0 @@
-From 8f60fc6ec611aa6ad6fa31f3dfc8027462dbb442 Mon Sep 17 00:00:00 2001
-From: Frediano Ziglio <fziglio@redhat.com>
-Date: Tue, 8 Sep 2015 13:06:03 +0100
-Subject: [PATCH 53/57] Fix some possible overflows in red_get_string for 32
- bit
-
-Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
-Acked-by: Christophe Fergeau <cfergeau@redhat.com>
----
- server/red_parse_qxl.c | 8 +++++++-
- 1 file changed, 7 insertions(+), 1 deletion(-)
-
-diff --git a/server/red_parse_qxl.c b/server/red_parse_qxl.c
-index f183248..668ce10 100644
---- a/server/red_parse_qxl.c
-+++ b/server/red_parse_qxl.c
-@@ -895,6 +895,11 @@ static SpiceString *red_get_string(RedMemSlotInfo *slots, int group_id,
- glyphs++;
- glyph_size = start->height * ((start->width * bpp + 7u) / 8u);
- red_size += sizeof(SpiceRasterGlyph *) + SPICE_ALIGN(sizeof(SpiceRasterGlyph) + glyph_size, 4);
-+ /* do the test correctly, we know end - start->data[0] cannot
-+ * overflow, don't use start->data[glyph_size] to test for
-+ * buffer overflow as this on 32 bit can cause overflow
-+ * on the pointer arithmetic */
-+ spice_assert(glyph_size <= (char*) end - (char*) &start->data[0]);
- start = (QXLRasterGlyph*)(&start->data[glyph_size]);
- }
- spice_assert(start <= end);
-@@ -915,7 +920,8 @@ static SpiceString *red_get_string(RedMemSlotInfo *slots, int group_id,
- red_get_point_ptr(&glyph->render_pos, &start->render_pos);
- red_get_point_ptr(&glyph->glyph_origin, &start->glyph_origin);
- glyph_size = glyph->height * ((glyph->width * bpp + 7u) / 8u);
-- spice_assert((QXLRasterGlyph*)(&start->data[glyph_size]) <= end);
-+ /* see above for similar test */
-+ spice_assert(glyph_size <= (char*) end - (char*) &start->data[0]);
- memcpy(glyph->data, start->data, glyph_size);
- start = (QXLRasterGlyph*)(&start->data[glyph_size]);
- glyph = (SpiceRasterGlyph*)
---
-2.4.3
-
diff --git a/SOURCES/0054-Fix-race-in-red_get_image.patch b/SOURCES/0054-Fix-race-in-red_get_image.patch
new file mode 100644
index 0000000..ac9cc18
--- /dev/null
+++ b/SOURCES/0054-Fix-race-in-red_get_image.patch
@@ -0,0 +1,77 @@
+From e4112afbe0f55df29068933e471fe348f79d4f04 Mon Sep 17 00:00:00 2001
+From: Frediano Ziglio <fziglio@redhat.com>
+Date: Tue, 8 Sep 2015 10:04:10 +0100
+Subject: [PATCH 54/64] Fix race in red_get_image
+
+Do not read multiple times data from guest as this could be changed
+by other vcpu threads.
+This causes races and security problems if these data are used for
+buffer allocation or checks.
+
+Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
+Acked-by: Christophe Fergeau <cfergeau@redhat.com>
+---
+ server/red_parse_qxl.c | 18 ++++++++++--------
+ 1 file changed, 10 insertions(+), 8 deletions(-)
+
+diff --git a/server/red_parse_qxl.c b/server/red_parse_qxl.c
+index affd3a2..84ea526 100644
+--- a/server/red_parse_qxl.c
++++ b/server/red_parse_qxl.c
+@@ -397,6 +397,7 @@ static SpiceImage *red_get_image(RedMemSlotInfo *slots, int group_id,
+ uint64_t bitmap_size, size;
+ uint8_t qxl_flags;
+ int error;
++ QXLPHYSICAL palette;
+
+ if (addr == 0) {
+ return NULL;
+@@ -422,12 +423,16 @@ static SpiceImage *red_get_image(RedMemSlotInfo *slots, int group_id,
+ switch (red->descriptor.type) {
+ case SPICE_IMAGE_TYPE_BITMAP:
+ red->u.bitmap.format = qxl->bitmap.format;
+- if (!bitmap_fmt_is_rgb(qxl->bitmap.format) && !qxl->bitmap.palette && !is_mask) {
++ red->u.bitmap.x = qxl->bitmap.x;
++ red->u.bitmap.y = qxl->bitmap.y;
++ red->u.bitmap.stride = qxl->bitmap.stride;
++ palette = qxl->bitmap.palette;
++ if (!bitmap_fmt_is_rgb(red->u.bitmap.format) && !palette && !is_mask) {
+ spice_warning("guest error: missing palette on bitmap format=%d\n",
+ red->u.bitmap.format);
+ goto error;
+ }
+- if (qxl->bitmap.x == 0 || qxl->bitmap.y == 0) {
++ if (red->u.bitmap.x == 0 || red->u.bitmap.y == 0) {
+ spice_warning("guest error: zero area bitmap\n");
+ goto error;
+ }
+@@ -435,23 +440,20 @@ static SpiceImage *red_get_image(RedMemSlotInfo *slots, int group_id,
+ if (qxl_flags & QXL_BITMAP_TOP_DOWN) {
+ red->u.bitmap.flags = SPICE_BITMAP_FLAGS_TOP_DOWN;
+ }
+- red->u.bitmap.x = qxl->bitmap.x;
+- red->u.bitmap.y = qxl->bitmap.y;
+- red->u.bitmap.stride = qxl->bitmap.stride;
+ if (!bitmap_consistent(&red->u.bitmap)) {
+ goto error;
+ }
+- if (qxl->bitmap.palette) {
++ if (palette) {
+ QXLPalette *qp;
+ int i, num_ents;
+- qp = (QXLPalette *)get_virt(slots, qxl->bitmap.palette,
++ qp = (QXLPalette *)get_virt(slots, palette,
+ sizeof(*qp), group_id, &error);
+ if (error) {
+ goto error;
+ }
+ num_ents = qp->num_ents;
+ if (!validate_virt(slots, (intptr_t)qp->ents,
+- get_memslot_id(slots, qxl->bitmap.palette),
++ get_memslot_id(slots, palette),
+ num_ents * sizeof(qp->ents[0]), group_id)) {
+ goto error;
+ }
+--
+2.4.3
+
diff --git a/SOURCES/0054-Make-sure-we-can-read-QXLPathSeg-structures.patch b/SOURCES/0054-Make-sure-we-can-read-QXLPathSeg-structures.patch
deleted file mode 100644
index f363968..0000000
--- a/SOURCES/0054-Make-sure-we-can-read-QXLPathSeg-structures.patch
+++ /dev/null
@@ -1,40 +0,0 @@
-From 289301f33c7da81fcb034448d96e8c276b4fc06a Mon Sep 17 00:00:00 2001
-From: Frediano Ziglio <fziglio@redhat.com>
-Date: Tue, 15 Sep 2015 16:25:17 +0100
-Subject: [PATCH 54/57] Make sure we can read QXLPathSeg structures
-
-start pointer points to a QXLPathSeg structure.
-Before reading from the structure, make sure the structure is contained
-in the memory range checked.
-
-Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
-Acked-by: Christophe Fergeau <cfergeau@redhat.com>
----
- server/red_parse_qxl.c | 4 ++--
- 1 file changed, 2 insertions(+), 2 deletions(-)
-
-diff --git a/server/red_parse_qxl.c b/server/red_parse_qxl.c
-index 668ce10..4663bfd 100644
---- a/server/red_parse_qxl.c
-+++ b/server/red_parse_qxl.c
-@@ -256,7 +256,7 @@ static SpicePath *red_get_path(RedMemSlotInfo *slots, int group_id,
-
- start = (QXLPathSeg*)data;
- end = (QXLPathSeg*)(data + size);
-- while (start < end) {
-+ while (start+1 < end) {
- n_segments++;
- count = start->count;
- segment_size = sizeof(SpicePathSeg) + count * sizeof(SpicePointFix);
-@@ -272,7 +272,7 @@ static SpicePath *red_get_path(RedMemSlotInfo *slots, int group_id,
- seg = (SpicePathSeg*)&red->segments[n_segments];
- n_segments = 0;
- mem_size2 = sizeof(*red);
-- while (start < end) {
-+ while (start+1 < end) {
- red->segments[n_segments++] = seg;
- count = start->count;
-
---
-2.4.3
-
diff --git a/SOURCES/0055-Avoid-race-condition-copying-segments-in-red_get_pat.patch b/SOURCES/0055-Avoid-race-condition-copying-segments-in-red_get_pat.patch
deleted file mode 100644
index 51c989c..0000000
--- a/SOURCES/0055-Avoid-race-condition-copying-segments-in-red_get_pat.patch
+++ /dev/null
@@ -1,31 +0,0 @@
-From 14f53eef04c38a3c537a1a1012c2f7101a298194 Mon Sep 17 00:00:00 2001
-From: Frediano Ziglio <fziglio@redhat.com>
-Date: Tue, 15 Sep 2015 16:38:23 +0100
-Subject: [PATCH 55/57] Avoid race condition copying segments in red_get_path
-
-The guest can attempt to increase the number of segments while
-spice-server is reading them.
-Make sure we don't copy more then the allocated segments.
-
-Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
-Acked-by: Christophe Fergeau <cfergeau@redhat.com>
----
- server/red_parse_qxl.c | 2 +-
- 1 file changed, 1 insertion(+), 1 deletion(-)
-
-diff --git a/server/red_parse_qxl.c b/server/red_parse_qxl.c
-index 4663bfd..c1df8e8 100644
---- a/server/red_parse_qxl.c
-+++ b/server/red_parse_qxl.c
-@@ -272,7 +272,7 @@ static SpicePath *red_get_path(RedMemSlotInfo *slots, int group_id,
- seg = (SpicePathSeg*)&red->segments[n_segments];
- n_segments = 0;
- mem_size2 = sizeof(*red);
-- while (start+1 < end) {
-+ while (start+1 < end && n_segments < red->num_segments) {
- red->segments[n_segments++] = seg;
- count = start->count;
-
---
-2.4.3
-
diff --git a/SOURCES/0055-Fix-race-condition-in-red_get_string.patch b/SOURCES/0055-Fix-race-condition-in-red_get_string.patch
new file mode 100644
index 0000000..a8a6cb4
--- /dev/null
+++ b/SOURCES/0055-Fix-race-condition-in-red_get_string.patch
@@ -0,0 +1,62 @@
+From ee52db855a3b25965d5e363d64831864efe3f4f3 Mon Sep 17 00:00:00 2001
+From: Frediano Ziglio <fziglio@redhat.com>
+Date: Tue, 8 Sep 2015 10:05:20 +0100
+Subject: [PATCH 55/64] Fix race condition in red_get_string
+
+Do not read multiple time an array size that can be changed.
+
+Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
+Acked-by: Christophe Fergeau <cfergeau@redhat.com>
+---
+ server/red_parse_qxl.c | 15 +++++++++------
+ 1 file changed, 9 insertions(+), 6 deletions(-)
+
+diff --git a/server/red_parse_qxl.c b/server/red_parse_qxl.c
+index 84ea526..2d4636e 100644
+--- a/server/red_parse_qxl.c
++++ b/server/red_parse_qxl.c
+@@ -809,6 +809,7 @@ static SpiceString *red_get_string(RedMemSlotInfo *slots, int group_id,
+ size_t chunk_size, qxl_size, red_size, glyph_size;
+ int glyphs, bpp = 0, i;
+ int error;
++ uint16_t qxl_flags, qxl_length;
+
+ qxl = (QXLString *)get_virt(slots, addr, sizeof(*qxl), group_id, &error);
+ if (error) {
+@@ -825,13 +826,15 @@ static SpiceString *red_get_string(RedMemSlotInfo *slots, int group_id,
+ red_put_data_chunks(&chunks);
+
+ qxl_size = qxl->data_size;
++ qxl_flags = qxl->flags;
++ qxl_length = qxl->length;
+ spice_assert(chunk_size == qxl_size);
+
+- if (qxl->flags & SPICE_STRING_FLAGS_RASTER_A1) {
++ if (qxl_flags & SPICE_STRING_FLAGS_RASTER_A1) {
+ bpp = 1;
+- } else if (qxl->flags & SPICE_STRING_FLAGS_RASTER_A4) {
++ } else if (qxl_flags & SPICE_STRING_FLAGS_RASTER_A4) {
+ bpp = 4;
+- } else if (qxl->flags & SPICE_STRING_FLAGS_RASTER_A8) {
++ } else if (qxl_flags & SPICE_STRING_FLAGS_RASTER_A8) {
+ bpp = 8;
+ }
+ spice_assert(bpp != 0);
+@@ -848,11 +851,11 @@ static SpiceString *red_get_string(RedMemSlotInfo *slots, int group_id,
+ start = (QXLRasterGlyph*)(&start->data[glyph_size]);
+ }
+ spice_assert(start <= end);
+- spice_assert(glyphs == qxl->length);
++ spice_assert(glyphs == qxl_length);
+
+ red = spice_malloc(red_size);
+- red->length = qxl->length;
+- red->flags = qxl->flags;
++ red->length = qxl_length;
++ red->flags = qxl_flags;
+
+ start = (QXLRasterGlyph*)data;
+ end = (QXLRasterGlyph*)(data + chunk_size);
+--
+2.4.3
+
diff --git a/SOURCES/0056-Fix-integer-overflow-computing-glyph_size-in-red_get.patch b/SOURCES/0056-Fix-integer-overflow-computing-glyph_size-in-red_get.patch
new file mode 100644
index 0000000..24c253f
--- /dev/null
+++ b/SOURCES/0056-Fix-integer-overflow-computing-glyph_size-in-red_get.patch
@@ -0,0 +1,63 @@
+From d6307f37304bddf6d76c42a6bdd8475ccab9582d Mon Sep 17 00:00:00 2001
+From: Frediano Ziglio <fziglio@redhat.com>
+Date: Tue, 8 Sep 2015 10:13:24 +0100
+Subject: [PATCH 56/64] Fix integer overflow computing glyph_size in
+ red_get_string
+
+If bpp is int the formula can lead to weird overflows. width and height
+are uint16_t so the formula is:
+
+ size_t = u16 * (u16 * int + const_int) / const_int;
+
+so it became
+
+ size_t = (int) u16 * ((int) u16 * int + const_int) / const_int;
+
+However the (int) u16 * (int) u16 can then became negative to overflow.
+Under 64 bit architectures size_t is 64 and int usually 32 so converting
+this negative 32 bit number to a unsigned 64 bit lead to a very big
+number as the signed is extended and then converted to unsigned.
+Using unsigned arithmetic prevent extending the sign.
+
+Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
+Acked-by: Christophe Fergeau <cfergeau@redhat.com>
+---
+ server/red_parse_qxl.c | 8 +++++---
+ 1 file changed, 5 insertions(+), 3 deletions(-)
+
+diff --git a/server/red_parse_qxl.c b/server/red_parse_qxl.c
+index 2d4636e..c4b82be 100644
+--- a/server/red_parse_qxl.c
++++ b/server/red_parse_qxl.c
+@@ -807,7 +807,9 @@ static SpiceString *red_get_string(RedMemSlotInfo *slots, int group_id,
+ uint8_t *data;
+ bool free_data;
+ size_t chunk_size, qxl_size, red_size, glyph_size;
+- int glyphs, bpp = 0, i;
++ int glyphs, i;
++ /* use unsigned to prevent integer overflow in multiplication below */
++ unsigned int bpp = 0;
+ int error;
+ uint16_t qxl_flags, qxl_length;
+
+@@ -846,7 +848,7 @@ static SpiceString *red_get_string(RedMemSlotInfo *slots, int group_id,
+ while (start < end) {
+ spice_assert((QXLRasterGlyph*)(&start->data[0]) <= end);
+ glyphs++;
+- glyph_size = start->height * ((start->width * bpp + 7) / 8);
++ glyph_size = start->height * ((start->width * bpp + 7u) / 8u);
+ red_size += sizeof(SpiceRasterGlyph *) + SPICE_ALIGN(sizeof(SpiceRasterGlyph) + glyph_size, 4);
+ start = (QXLRasterGlyph*)(&start->data[glyph_size]);
+ }
+@@ -867,7 +869,7 @@ static SpiceString *red_get_string(RedMemSlotInfo *slots, int group_id,
+ glyph->height = start->height;
+ red_get_point_ptr(&glyph->render_pos, &start->render_pos);
+ red_get_point_ptr(&glyph->glyph_origin, &start->glyph_origin);
+- glyph_size = glyph->height * ((glyph->width * bpp + 7) / 8);
++ glyph_size = glyph->height * ((glyph->width * bpp + 7u) / 8u);
+ spice_assert((QXLRasterGlyph*)(&start->data[glyph_size]) <= end);
+ memcpy(glyph->data, start->data, glyph_size);
+ start = (QXLRasterGlyph*)(&start->data[glyph_size]);
+--
+2.4.3
+
diff --git a/SOURCES/0056-Prevent-data_size-to-be-set-independently-from-data.patch b/SOURCES/0056-Prevent-data_size-to-be-set-independently-from-data.patch
deleted file mode 100644
index a78ea9a..0000000
--- a/SOURCES/0056-Prevent-data_size-to-be-set-independently-from-data.patch
+++ /dev/null
@@ -1,29 +0,0 @@
-From c2cdd1daf8edceec8adbb456dca656efe3648eec Mon Sep 17 00:00:00 2001
-From: Frediano Ziglio <fziglio@redhat.com>
-Date: Thu, 17 Sep 2015 14:28:36 +0100
-Subject: [PATCH 56/57] Prevent data_size to be set independently from data
-
-There was not check for data_size field so one could set data to
-a small set of data and data_size much bigger than size of data
-leading to buffer overflow.
-
-Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
----
- server/red_parse_qxl.c | 1 +
- 1 file changed, 1 insertion(+)
-
-diff --git a/server/red_parse_qxl.c b/server/red_parse_qxl.c
-index c1df8e8..8e3dd55 100644
---- a/server/red_parse_qxl.c
-+++ b/server/red_parse_qxl.c
-@@ -1391,6 +1391,7 @@ static int red_get_cursor(RedMemSlotInfo *slots, int group_id,
- size = red_get_data_chunks_ptr(slots, group_id,
- get_memslot_id(slots, addr),
- &chunks, &qxl->chunk);
-+ red->data_size = MIN(red->data_size, size);
- data = red_linearize_chunk(&chunks, size, &free_data);
- red_put_data_chunks(&chunks);
- if (free_data) {
---
-2.4.3
-
diff --git a/SOURCES/0057-Fix-race-condition-in-red_get_data_chunks_ptr.patch b/SOURCES/0057-Fix-race-condition-in-red_get_data_chunks_ptr.patch
new file mode 100644
index 0000000..d128555
--- /dev/null
+++ b/SOURCES/0057-Fix-race-condition-in-red_get_data_chunks_ptr.patch
@@ -0,0 +1,66 @@
+From a1ead07b6a0facefafb959542f3088f427c7eb2d Mon Sep 17 00:00:00 2001
+From: Frediano Ziglio <fziglio@redhat.com>
+Date: Tue, 8 Sep 2015 12:12:19 +0100
+Subject: [PATCH 57/64] Fix race condition in red_get_data_chunks_ptr
+
+Do not read multiple times data from guest as this can be changed by
+other guest vcpus. This causes races and security problems if these
+data are used for buffer allocation or checks.
+
+Actually, the 'data' member can't change during read as it is just a
+pointer to a fixed array contained in qxl. However, this change will
+make it clear that there can be no race condition.
+
+Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
+---
+ server/red_parse_qxl.c | 17 ++++++++++-------
+ 1 file changed, 10 insertions(+), 7 deletions(-)
+
+diff --git a/server/red_parse_qxl.c b/server/red_parse_qxl.c
+index c4b82be..7cc20e6 100644
+--- a/server/red_parse_qxl.c
++++ b/server/red_parse_qxl.c
+@@ -102,30 +102,33 @@ static size_t red_get_data_chunks_ptr(RedMemSlotInfo *slots, int group_id,
+ RedDataChunk *red_prev;
+ size_t data_size = 0;
+ int error;
++ QXLPHYSICAL next_chunk;
+
+ red->data_size = qxl->data_size;
+ data_size += red->data_size;
+- if (!validate_virt(slots, (intptr_t)qxl->data, memslot_id, red->data_size, group_id)) {
++ red->data = qxl->data;
++ if (!validate_virt(slots, (intptr_t)red->data, memslot_id, red->data_size, group_id)) {
++ red->data = NULL;
+ return 0;
+ }
+- red->data = qxl->data;
+ red->prev_chunk = NULL;
+
+- while (qxl->next_chunk) {
++ while ((next_chunk = qxl->next_chunk) != 0) {
+ red_prev = red;
+ red = spice_new(RedDataChunk, 1);
+- memslot_id = get_memslot_id(slots, qxl->next_chunk);
+- qxl = (QXLDataChunk *)get_virt(slots, qxl->next_chunk, sizeof(*qxl), group_id,
++ memslot_id = get_memslot_id(slots, next_chunk);
++ qxl = (QXLDataChunk *)get_virt(slots, next_chunk, sizeof(*qxl), group_id,
+ &error);
+ if (error) {
+ return 0;
+ }
+ red->data_size = qxl->data_size;
+ data_size += red->data_size;
+- if (!validate_virt(slots, (intptr_t)qxl->data, memslot_id, red->data_size, group_id)) {
++ red->data = qxl->data;
++ if (!validate_virt(slots, (intptr_t)red->data, memslot_id, red->data_size, group_id)) {
++ red->data = NULL;
+ return 0;
+ }
+- red->data = qxl->data;
+ red->prev_chunk = red_prev;
+ red_prev->next_chunk = red;
+ }
+--
+2.4.3
+
diff --git a/SOURCES/0057-Prevent-leak-if-size-from-red_get_data_chunks-don-t-.patch b/SOURCES/0057-Prevent-leak-if-size-from-red_get_data_chunks-don-t-.patch
deleted file mode 100644
index b7bf58f..0000000
--- a/SOURCES/0057-Prevent-leak-if-size-from-red_get_data_chunks-don-t-.patch
+++ /dev/null
@@ -1,34 +0,0 @@
-From 5580cac5502cd518adad0a3682fd53aeeaf86a86 Mon Sep 17 00:00:00 2001
-From: Frediano Ziglio <fziglio@redhat.com>
-Date: Thu, 17 Sep 2015 15:01:05 +0100
-Subject: [PATCH 57/57] Prevent leak if size from red_get_data_chunks don't
- match in red_get_image
-
-Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
----
- server/red_parse_qxl.c | 2 ++
- 1 file changed, 2 insertions(+)
-
-diff --git a/server/red_parse_qxl.c b/server/red_parse_qxl.c
-index 8e3dd55..bd0c408 100644
---- a/server/red_parse_qxl.c
-+++ b/server/red_parse_qxl.c
-@@ -530,6 +530,7 @@ static SpiceImage *red_get_image(RedMemSlotInfo *slots, int group_id,
- &chunks, qxl->bitmap.data);
- spice_assert(size == bitmap_size);
- if (size != bitmap_size) {
-+ red_put_data_chunks(&chunks);
- goto error;
- }
- red->u.bitmap.data = red_get_image_data_chunked(slots, group_id,
-@@ -550,6 +551,7 @@ static SpiceImage *red_get_image(RedMemSlotInfo *slots, int group_id,
- &chunks, (QXLDataChunk *)qxl->quic.data);
- spice_assert(size == red->u.quic.data_size);
- if (size != red->u.quic.data_size) {
-+ red_put_data_chunks(&chunks);
- goto error;
- }
- red->u.quic.data = red_get_image_data_chunked(slots, group_id,
---
-2.4.3
-
diff --git a/SOURCES/0058-Prevent-memory-leak-if-red_get_data_chunks_ptr-fails.patch b/SOURCES/0058-Prevent-memory-leak-if-red_get_data_chunks_ptr-fails.patch
new file mode 100644
index 0000000..f28db57
--- /dev/null
+++ b/SOURCES/0058-Prevent-memory-leak-if-red_get_data_chunks_ptr-fails.patch
@@ -0,0 +1,75 @@
+From 098e6e9e479a1188c5b5050ac8a0d4f495ec4842 Mon Sep 17 00:00:00 2001
+From: Frediano Ziglio <fziglio@redhat.com>
+Date: Tue, 8 Sep 2015 12:14:55 +0100
+Subject: [PATCH 58/64] Prevent memory leak if red_get_data_chunks_ptr fails
+
+Free linked list if client tries to do nasty things
+
+Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
+Acked-by: Christophe Fergeau <cfergeau@redhat.com>
+---
+ server/red_parse_qxl.c | 31 ++++++++++++++++++++-----------
+ 1 file changed, 20 insertions(+), 11 deletions(-)
+
+diff --git a/server/red_parse_qxl.c b/server/red_parse_qxl.c
+index 7cc20e6..fe3ae78 100644
+--- a/server/red_parse_qxl.c
++++ b/server/red_parse_qxl.c
+@@ -107,34 +107,43 @@ static size_t red_get_data_chunks_ptr(RedMemSlotInfo *slots, int group_id,
+ red->data_size = qxl->data_size;
+ data_size += red->data_size;
+ red->data = qxl->data;
++ red->prev_chunk = red->next_chunk = NULL;
+ if (!validate_virt(slots, (intptr_t)red->data, memslot_id, red->data_size, group_id)) {
+ red->data = NULL;
+ return 0;
+ }
+- red->prev_chunk = NULL;
+
+ while ((next_chunk = qxl->next_chunk) != 0) {
+ red_prev = red;
+- red = spice_new(RedDataChunk, 1);
++ red = spice_new0(RedDataChunk, 1);
++ red->prev_chunk = red_prev;
++ red_prev->next_chunk = red;
++
+ memslot_id = get_memslot_id(slots, next_chunk);
+ qxl = (QXLDataChunk *)get_virt(slots, next_chunk, sizeof(*qxl), group_id,
+ &error);
+- if (error) {
+- return 0;
+- }
++ if (error)
++ goto error;
+ red->data_size = qxl->data_size;
+ data_size += red->data_size;
+ red->data = qxl->data;
+- if (!validate_virt(slots, (intptr_t)red->data, memslot_id, red->data_size, group_id)) {
+- red->data = NULL;
+- return 0;
+- }
+- red->prev_chunk = red_prev;
+- red_prev->next_chunk = red;
++ if (!validate_virt(slots, (intptr_t)red->data, memslot_id, red->data_size, group_id))
++ goto error;
+ }
+
+ red->next_chunk = NULL;
+ return data_size;
++
++error:
++ while (red->prev_chunk) {
++ red_prev = red->prev_chunk;
++ free(red);
++ red = red_prev;
++ }
++ red->data_size = 0;
++ red->next_chunk = NULL;
++ red->data = NULL;
++ return 0;
+ }
+
+ static size_t red_get_data_chunks(RedMemSlotInfo *slots, int group_id,
+--
+2.4.3
+
diff --git a/SOURCES/0059-Prevent-DoS-from-guest-trying-to-allocate-too-much-d.patch b/SOURCES/0059-Prevent-DoS-from-guest-trying-to-allocate-too-much-d.patch
new file mode 100644
index 0000000..2f11b59
--- /dev/null
+++ b/SOURCES/0059-Prevent-DoS-from-guest-trying-to-allocate-too-much-d.patch
@@ -0,0 +1,102 @@
+From f89da55f4c4fcc09f201562854f3768911fdab07 Mon Sep 17 00:00:00 2001
+From: Frediano Ziglio <fziglio@redhat.com>
+Date: Tue, 8 Sep 2015 12:28:54 +0100
+Subject: [PATCH 59/64] Prevent DoS from guest trying to allocate too much data
+ on host for chunks
+
+Limit number of chunks to a given amount to avoid guest trying to
+allocate too much memory. Using circular or nested chunks lists
+guest could try to allocate huge amounts of memory.
+Considering the list can be infinite and guest can change data this
+also prevents strange security attacks from guest.
+
+Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
+---
+ server/red_parse_qxl.c | 49 +++++++++++++++++++++++++++++++++++++++++--------
+ 1 file changed, 41 insertions(+), 8 deletions(-)
+
+diff --git a/server/red_parse_qxl.c b/server/red_parse_qxl.c
+index fe3ae78..f183248 100644
+--- a/server/red_parse_qxl.c
++++ b/server/red_parse_qxl.c
+@@ -37,6 +37,13 @@
+
+ G_STATIC_ASSERT(MAX_DATA_CHUNK <= G_MAXINT32);
+
++/* Limit number of chunks.
++ * The guest can attempt to make host allocate too much memory
++ * just with a large number of small chunks.
++ * Prevent that the chunk list take more memory than the data itself.
++ */
++#define MAX_CHUNKS (MAX_DATA_CHUNK/1024u)
++
+ #if 0
+ static void hexdump_qxl(RedMemSlotInfo *slots, int group_id,
+ QXLPHYSICAL addr, uint8_t bytes)
+@@ -100,9 +107,11 @@ static size_t red_get_data_chunks_ptr(RedMemSlotInfo *slots, int group_id,
+ RedDataChunk *red, QXLDataChunk *qxl)
+ {
+ RedDataChunk *red_prev;
+- size_t data_size = 0;
++ uint64_t data_size = 0;
++ uint32_t chunk_data_size;
+ int error;
+ QXLPHYSICAL next_chunk;
++ unsigned num_chunks = 0;
+
+ red->data_size = qxl->data_size;
+ data_size += red->data_size;
+@@ -114,19 +123,43 @@ static size_t red_get_data_chunks_ptr(RedMemSlotInfo *slots, int group_id,
+ }
+
+ while ((next_chunk = qxl->next_chunk) != 0) {
++ /* somebody is trying to use too much memory using a lot of chunks.
++ * Or made a circular list of chunks
++ */
++ if (++num_chunks >= MAX_CHUNKS) {
++ spice_warning("data split in too many chunks, avoiding DoS\n");
++ goto error;
++ }
++
++ memslot_id = get_memslot_id(slots, next_chunk);
++ qxl = (QXLDataChunk *)get_virt(slots, next_chunk, sizeof(*qxl),
++ group_id, &error);
++ if (error)
++ goto error;
++
++ /* do not waste space for empty chunks.
++ * This could be just a driver issue or an attempt
++ * to allocate too much memory or a circular list.
++ * All above cases are handled by the check for number
++ * of chunks.
++ */
++ chunk_data_size = qxl->data_size;
++ if (chunk_data_size == 0)
++ continue;
++
+ red_prev = red;
+ red = spice_new0(RedDataChunk, 1);
++ red->data_size = chunk_data_size;
+ red->prev_chunk = red_prev;
++ red->data = qxl->data;
+ red_prev->next_chunk = red;
+
+- memslot_id = get_memslot_id(slots, next_chunk);
+- qxl = (QXLDataChunk *)get_virt(slots, next_chunk, sizeof(*qxl), group_id,
+- &error);
+- if (error)
++ data_size += chunk_data_size;
++ /* this can happen if client is sending nested chunks */
++ if (data_size > MAX_DATA_CHUNK) {
++ spice_warning("too much data inside chunks, avoiding DoS\n");
+ goto error;
+- red->data_size = qxl->data_size;
+- data_size += red->data_size;
+- red->data = qxl->data;
++ }
+ if (!validate_virt(slots, (intptr_t)red->data, memslot_id, red->data_size, group_id))
+ goto error;
+ }
+--
+2.4.3
+
diff --git a/SOURCES/0060-Fix-some-possible-overflows-in-red_get_string-for-32.patch b/SOURCES/0060-Fix-some-possible-overflows-in-red_get_string-for-32.patch
new file mode 100644
index 0000000..b94f03f
--- /dev/null
+++ b/SOURCES/0060-Fix-some-possible-overflows-in-red_get_string-for-32.patch
@@ -0,0 +1,41 @@
+From b61b33beae8b7560d05fe0d8349446bc16d08c62 Mon Sep 17 00:00:00 2001
+From: Frediano Ziglio <fziglio@redhat.com>
+Date: Tue, 8 Sep 2015 13:06:03 +0100
+Subject: [PATCH 60/64] Fix some possible overflows in red_get_string for 32
+ bit
+
+Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
+Acked-by: Christophe Fergeau <cfergeau@redhat.com>
+---
+ server/red_parse_qxl.c | 8 +++++++-
+ 1 file changed, 7 insertions(+), 1 deletion(-)
+
+diff --git a/server/red_parse_qxl.c b/server/red_parse_qxl.c
+index f183248..668ce10 100644
+--- a/server/red_parse_qxl.c
++++ b/server/red_parse_qxl.c
+@@ -895,6 +895,11 @@ static SpiceString *red_get_string(RedMemSlotInfo *slots, int group_id,
+ glyphs++;
+ glyph_size = start->height * ((start->width * bpp + 7u) / 8u);
+ red_size += sizeof(SpiceRasterGlyph *) + SPICE_ALIGN(sizeof(SpiceRasterGlyph) + glyph_size, 4);
++ /* do the test correctly, we know end - start->data[0] cannot
++ * overflow, don't use start->data[glyph_size] to test for
++ * buffer overflow as this on 32 bit can cause overflow
++ * on the pointer arithmetic */
++ spice_assert(glyph_size <= (char*) end - (char*) &start->data[0]);
+ start = (QXLRasterGlyph*)(&start->data[glyph_size]);
+ }
+ spice_assert(start <= end);
+@@ -915,7 +920,8 @@ static SpiceString *red_get_string(RedMemSlotInfo *slots, int group_id,
+ red_get_point_ptr(&glyph->render_pos, &start->render_pos);
+ red_get_point_ptr(&glyph->glyph_origin, &start->glyph_origin);
+ glyph_size = glyph->height * ((glyph->width * bpp + 7u) / 8u);
+- spice_assert((QXLRasterGlyph*)(&start->data[glyph_size]) <= end);
++ /* see above for similar test */
++ spice_assert(glyph_size <= (char*) end - (char*) &start->data[0]);
+ memcpy(glyph->data, start->data, glyph_size);
+ start = (QXLRasterGlyph*)(&start->data[glyph_size]);
+ glyph = (SpiceRasterGlyph*)
+--
+2.4.3
+
diff --git a/SOURCES/0061-Make-sure-we-can-read-QXLPathSeg-structures.patch b/SOURCES/0061-Make-sure-we-can-read-QXLPathSeg-structures.patch
new file mode 100644
index 0000000..ad6bc16
--- /dev/null
+++ b/SOURCES/0061-Make-sure-we-can-read-QXLPathSeg-structures.patch
@@ -0,0 +1,40 @@
+From 13dadd08709caa8e736cbbc886f5f8ba7bad7785 Mon Sep 17 00:00:00 2001
+From: Frediano Ziglio <fziglio@redhat.com>
+Date: Tue, 15 Sep 2015 16:25:17 +0100
+Subject: [PATCH 61/64] Make sure we can read QXLPathSeg structures
+
+start pointer points to a QXLPathSeg structure.
+Before reading from the structure, make sure the structure is contained
+in the memory range checked.
+
+Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
+Acked-by: Christophe Fergeau <cfergeau@redhat.com>
+---
+ server/red_parse_qxl.c | 4 ++--
+ 1 file changed, 2 insertions(+), 2 deletions(-)
+
+diff --git a/server/red_parse_qxl.c b/server/red_parse_qxl.c
+index 668ce10..4663bfd 100644
+--- a/server/red_parse_qxl.c
++++ b/server/red_parse_qxl.c
+@@ -256,7 +256,7 @@ static SpicePath *red_get_path(RedMemSlotInfo *slots, int group_id,
+
+ start = (QXLPathSeg*)data;
+ end = (QXLPathSeg*)(data + size);
+- while (start < end) {
++ while (start+1 < end) {
+ n_segments++;
+ count = start->count;
+ segment_size = sizeof(SpicePathSeg) + count * sizeof(SpicePointFix);
+@@ -272,7 +272,7 @@ static SpicePath *red_get_path(RedMemSlotInfo *slots, int group_id,
+ seg = (SpicePathSeg*)&red->segments[n_segments];
+ n_segments = 0;
+ mem_size2 = sizeof(*red);
+- while (start < end) {
++ while (start+1 < end) {
+ red->segments[n_segments++] = seg;
+ count = start->count;
+
+--
+2.4.3
+
diff --git a/SOURCES/0062-Avoid-race-condition-copying-segments-in-red_get_pat.patch b/SOURCES/0062-Avoid-race-condition-copying-segments-in-red_get_pat.patch
new file mode 100644
index 0000000..25da97b
--- /dev/null
+++ b/SOURCES/0062-Avoid-race-condition-copying-segments-in-red_get_pat.patch
@@ -0,0 +1,31 @@
+From a2510f4df1c01a48515504c25cd9f0d9d1e839d0 Mon Sep 17 00:00:00 2001
+From: Frediano Ziglio <fziglio@redhat.com>
+Date: Tue, 15 Sep 2015 16:38:23 +0100
+Subject: [PATCH 62/64] Avoid race condition copying segments in red_get_path
+
+The guest can attempt to increase the number of segments while
+spice-server is reading them.
+Make sure we don't copy more then the allocated segments.
+
+Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
+Acked-by: Christophe Fergeau <cfergeau@redhat.com>
+---
+ server/red_parse_qxl.c | 2 +-
+ 1 file changed, 1 insertion(+), 1 deletion(-)
+
+diff --git a/server/red_parse_qxl.c b/server/red_parse_qxl.c
+index 4663bfd..c1df8e8 100644
+--- a/server/red_parse_qxl.c
++++ b/server/red_parse_qxl.c
+@@ -272,7 +272,7 @@ static SpicePath *red_get_path(RedMemSlotInfo *slots, int group_id,
+ seg = (SpicePathSeg*)&red->segments[n_segments];
+ n_segments = 0;
+ mem_size2 = sizeof(*red);
+- while (start+1 < end) {
++ while (start+1 < end && n_segments < red->num_segments) {
+ red->segments[n_segments++] = seg;
+ count = start->count;
+
+--
+2.4.3
+
diff --git a/SOURCES/0063-Prevent-data_size-to-be-set-independently-from-data.patch b/SOURCES/0063-Prevent-data_size-to-be-set-independently-from-data.patch
new file mode 100644
index 0000000..1ccbb11
--- /dev/null
+++ b/SOURCES/0063-Prevent-data_size-to-be-set-independently-from-data.patch
@@ -0,0 +1,29 @@
+From acda240cbeb8a15aafd73b55a22fd3dfa7f03df1 Mon Sep 17 00:00:00 2001
+From: Frediano Ziglio <fziglio@redhat.com>
+Date: Thu, 17 Sep 2015 14:28:36 +0100
+Subject: [PATCH 63/64] Prevent data_size to be set independently from data
+
+There was not check for data_size field so one could set data to
+a small set of data and data_size much bigger than size of data
+leading to buffer overflow.
+
+Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
+---
+ server/red_parse_qxl.c | 1 +
+ 1 file changed, 1 insertion(+)
+
+diff --git a/server/red_parse_qxl.c b/server/red_parse_qxl.c
+index c1df8e8..8e3dd55 100644
+--- a/server/red_parse_qxl.c
++++ b/server/red_parse_qxl.c
+@@ -1391,6 +1391,7 @@ static int red_get_cursor(RedMemSlotInfo *slots, int group_id,
+ size = red_get_data_chunks_ptr(slots, group_id,
+ get_memslot_id(slots, addr),
+ &chunks, &qxl->chunk);
++ red->data_size = MIN(red->data_size, size);
+ data = red_linearize_chunk(&chunks, size, &free_data);
+ red_put_data_chunks(&chunks);
+ if (free_data) {
+--
+2.4.3
+
diff --git a/SOURCES/0064-Prevent-leak-if-size-from-red_get_data_chunks-don-t-.patch b/SOURCES/0064-Prevent-leak-if-size-from-red_get_data_chunks-don-t-.patch
new file mode 100644
index 0000000..7b1a381
--- /dev/null
+++ b/SOURCES/0064-Prevent-leak-if-size-from-red_get_data_chunks-don-t-.patch
@@ -0,0 +1,34 @@
+From ffa092aa729613de9fe9f7f59beea4338b324f5c Mon Sep 17 00:00:00 2001
+From: Frediano Ziglio <fziglio@redhat.com>
+Date: Thu, 17 Sep 2015 15:01:05 +0100
+Subject: [PATCH 64/64] Prevent leak if size from red_get_data_chunks don't
+ match in red_get_image
+
+Signed-off-by: Frediano Ziglio <fziglio@redhat.com>
+---
+ server/red_parse_qxl.c | 2 ++
+ 1 file changed, 2 insertions(+)
+
+diff --git a/server/red_parse_qxl.c b/server/red_parse_qxl.c
+index 8e3dd55..bd0c408 100644
+--- a/server/red_parse_qxl.c
++++ b/server/red_parse_qxl.c
+@@ -530,6 +530,7 @@ static SpiceImage *red_get_image(RedMemSlotInfo *slots, int group_id,
+ &chunks, qxl->bitmap.data);
+ spice_assert(size == bitmap_size);
+ if (size != bitmap_size) {
++ red_put_data_chunks(&chunks);
+ goto error;
+ }
+ red->u.bitmap.data = red_get_image_data_chunked(slots, group_id,
+@@ -550,6 +551,7 @@ static SpiceImage *red_get_image(RedMemSlotInfo *slots, int group_id,
+ &chunks, (QXLDataChunk *)qxl->quic.data);
+ spice_assert(size == red->u.quic.data_size);
+ if (size != red->u.quic.data_size) {
++ red_put_data_chunks(&chunks);
+ goto error;
+ }
+ red->u.quic.data = red_get_image_data_chunked(slots, group_id,
+--
+2.4.3
+
diff --git a/SPECS/spice.spec b/SPECS/spice.spec
index f24555c..25eeb81 100644
--- a/SPECS/spice.spec
+++ b/SPECS/spice.spec
@@ -1,6 +1,6 @@
Name: spice
Version: 0.12.4
-Release: 9%{?dist}.3
+Release: 15%{?dist}
Summary: Implements the SPICE protocol
Group: User Interface/Desktops
License: LGPLv2+
@@ -43,26 +43,34 @@ Patch34: 0034-Validate-surface-bounding-box-before-using-it.patch
Patch35: 0035-migration-Don-t-assert-if-MIGRATE_DATA-comes-before-.patch
Patch36: 0036-server-fix-crash-when-restarting-VM-with-old-client.patch
Patch37: 0037-Use-TLS-version-1.0-or-better.patch
-Patch38: 0038-Avoid-race-conditions-reading-monitor-configs-from-g.patch
-Patch39: 0039-worker-validate-correctly-surfaces.patch
-Patch40: 0040-worker-avoid-double-free-or-double-create-of-surface.patch
-Patch41: 0041-Define-a-constant-to-limit-data-from-guest.patch
-Patch42: 0042-Fix-some-integer-overflow-causing-large-memory-alloc.patch
-Patch43: 0043-Check-properly-surface-to-be-created.patch
-Patch44: 0044-Fix-buffer-reading-overflow.patch
-Patch45: 0045-Prevent-32-bit-integer-overflow-in-bitmap_consistent.patch
-Patch46: 0046-Fix-race-condition-on-red_get_clip_rects.patch
-Patch47: 0047-Fix-race-in-red_get_image.patch
-Patch48: 0048-Fix-race-condition-in-red_get_string.patch
-Patch49: 0049-Fix-integer-overflow-computing-glyph_size-in-red_get.patch
-Patch50: 0050-Fix-race-condition-in-red_get_data_chunks_ptr.patch
-Patch51: 0051-Prevent-memory-leak-if-red_get_data_chunks_ptr-fails.patch
-Patch52: 0052-Prevent-DoS-from-guest-trying-to-allocate-too-much-d.patch
-Patch53: 0053-Fix-some-possible-overflows-in-red_get_string-for-32.patch
-Patch54: 0054-Make-sure-we-can-read-QXLPathSeg-structures.patch
-Patch55: 0055-Avoid-race-condition-copying-segments-in-red_get_pat.patch
-Patch56: 0056-Prevent-data_size-to-be-set-independently-from-data.patch
-Patch57: 0057-Prevent-leak-if-size-from-red_get_data_chunks-don-t-.patch
+Patch38: 0038-Add-const-to-test_capability-first-argument.patch
+Patch39: 0039-Introduce-red_link_info_test_capability.patch
+Patch40: 0040-Don-t-set-SpiceLinkReply-pub_key-if-client-advertise.patch
+Patch41: 0041-server-don-t-assert-on-invalid-client-message.patch
+Patch42: 0042-Don-t-truncate-large-now-values-in-_spice_timer_set.patch
+Patch43: 0043-Avoid-race-conditions-reading-monitor-configs-from-g.patch
+Patch44: 0044-Lock-the-pixmap-image-cache-for-the-entire-fill_bits.patch
+Patch45: 0045-reds-Assure-we-don-t-have-stale-statistic-files-befo.patch
+Patch46: 0046-worker-validate-correctly-surfaces.patch
+Patch47: 0047-worker-avoid-double-free-or-double-create-of-surface.patch
+Patch48: 0048-Define-a-constant-to-limit-data-from-guest.patch
+Patch49: 0049-Fix-some-integer-overflow-causing-large-memory-alloc.patch
+Patch50: 0050-Check-properly-surface-to-be-created.patch
+Patch51: 0051-Fix-buffer-reading-overflow.patch
+Patch52: 0052-Prevent-32-bit-integer-overflow-in-bitmap_consistent.patch
+Patch53: 0053-Fix-race-condition-on-red_get_clip_rects.patch
+Patch54: 0054-Fix-race-in-red_get_image.patch
+Patch55: 0055-Fix-race-condition-in-red_get_string.patch
+Patch56: 0056-Fix-integer-overflow-computing-glyph_size-in-red_get.patch
+Patch57: 0057-Fix-race-condition-in-red_get_data_chunks_ptr.patch
+Patch58: 0058-Prevent-memory-leak-if-red_get_data_chunks_ptr-fails.patch
+Patch59: 0059-Prevent-DoS-from-guest-trying-to-allocate-too-much-d.patch
+Patch60: 0060-Fix-some-possible-overflows-in-red_get_string-for-32.patch
+Patch61: 0061-Make-sure-we-can-read-QXLPathSeg-structures.patch
+Patch62: 0062-Avoid-race-condition-copying-segments-in-red_get_pat.patch
+Patch63: 0063-Prevent-data_size-to-be-set-independently-from-data.patch
+Patch64: 0064-Prevent-leak-if-size-from-red_get_data_chunks-don-t-.patch
+
# https://bugzilla.redhat.com/show_bug.cgi?id=613529
%if 0%{?rhel}
@@ -175,6 +183,13 @@ using spice-server, you will need to install spice-server-devel.
%patch55 -p1
%patch56 -p1
%patch57 -p1
+%patch58 -p1
+%patch59 -p1
+%patch60 -p1
+%patch61 -p1
+%patch62 -p1
+%patch63 -p1
+%patch64 -p1
%build
@@ -205,18 +220,37 @@ mkdir -p %{buildroot}%{_libexecdir}
%changelog
-* Wed Sep 23 2015 Frediano Ziglio <fziglio@redhat.com> 0.12.4-9.3
+* Wed Sep 23 2015 Frediano Ziglio <fziglio@redhat.com> 0.12.4-15
- CVE-2015-5260 CVE-2015-5261 fixed various security flaws
- Resolves: rhbz#1262771
+ Resolves: rhbz#1267134
-* Tue Sep 15 2015 Frediano Ziglio <fziglio@redhat.com> 0.12.4-9.2
+* Thu Sep 10 2015 Frediano Ziglio <fziglio@redhat.com> 0.12.4-14
- Validate surface_id
- Resolves: rhbz#1262771
-
-* Tue Jul 21 2015 Christophe Fergeau <cfergeau@redhat.com> 0.12.4-9.1
-- Avoid race conditions reading monitor configs from guest. This race could
- trigger memory corruption host-side
- Resolves: rhbz#1239127
+ Resolves: rhbz#1260971
+
+* Tue Jul 21 2015 Frediano Ziglio <fziglio@redhat.com> 0.12.4-13
+- Clean stale statistics file before creating a new one
+ Resolves: rhbz#1177326
+
+* Fri Jul 10 2015 Fabiano Fidêncio <fidencio@redhat.com> 0.12.4-12
+- Fix a backport issue on Patch0040.
+ Related: rhbz#1071176
+ Resolves: rhbz#1241860
+
+* Thu Jul 09 2015 Fabiano Fidêncio <fidencio@redhat.com> 0.12.4-11
+- Don't assert on invalid client message
+ Resolves: rhbz#1227410
+- Don't truncate large 'now' values in _spice_timer_set
+ Resolves: rhbz#1227408
+- Avoid race conditions reading monitor configs from guest
+ Resolves: rhbz#1239128
+- Lock the pixmap image cache for the entire fill_bits call
+ Resolves: rhbz#1235443
+
+* Wed Jul 08 2015 Fabiano Fidêncio <fidencio@redhat.com> 0.12.4-10
+- Fix qemu segmentation fault (core dumped) when boot KVM guest with
+ spice in FIPS enabled mode.
+ Resolves: rhbz#1071176
* Mon Jan 05 2015 Marc-Andre Lureau <marcandre.lureau@redhat.com> 0.12.4-9
- Allow recent TLS/SSL methods, block SSLv2/SSLv3. Resolves: rhbz#1175540