diff options
Diffstat (limited to 'meta')
| -rw-r--r-- | meta/recipes-devtools/qemu/qemu/CVE-2017-15124.patch | 1476 | ||||
| -rw-r--r-- | meta/recipes-devtools/qemu/qemu_2.11.0.bb | 1 |
2 files changed, 1477 insertions, 0 deletions
diff --git a/meta/recipes-devtools/qemu/qemu/CVE-2017-15124.patch b/meta/recipes-devtools/qemu/qemu/CVE-2017-15124.patch new file mode 100644 index 0000000000..a47b6d0510 --- /dev/null +++ b/meta/recipes-devtools/qemu/qemu/CVE-2017-15124.patch @@ -0,0 +1,1476 @@ +VNC server implementation in Quick Emulator (QEMU) 2.11.0 and older was found to +be vulnerable to an unbounded memory allocation issue, as it did not throttle +the framebuffer updates sent to its client. If the client did not consume these +updates, VNC server allocates growing memory to hold onto this data. A malicious +remote VNC client could use this flaw to cause DoS to the server host. + +CVE: CVE-2017-15124 +Upstream-Status: Backport +Signed-off-by: Ross Burton <ross.burton@intel.com> + +From 090fdc83b0960f68d204624a73c6814780da52d9 Mon Sep 17 00:00:00 2001 +From: =?UTF-8?q?Marc-Andr=C3=A9=20Lureau?= <marcandre.lureau@redhat.com> +Date: Wed, 20 Dec 2017 15:06:18 +0100 +Subject: [PATCH 01/14] vnc: fix debug spelling +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> +Message-id: 20171220140618.12701-1-marcandre.lureau@redhat.com +Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> +--- + ui/vnc.c | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/ui/vnc.c b/ui/vnc.c +index 9f8d5a1b1f..7d537b5c6b 100644 +--- a/ui/vnc.c ++++ b/ui/vnc.c +@@ -2255,7 +2255,7 @@ static int protocol_client_msg(VncState *vs, uint8_t *data, size_t len) + } + vs->as.nchannels = read_u8(data, 5); + if (vs->as.nchannels != 1 && vs->as.nchannels != 2) { +- VNC_DEBUG("Invalid audio channel coount %d\n", ++ VNC_DEBUG("Invalid audio channel count %d\n", + read_u8(data, 5)); + vnc_client_error(vs); + break; +-- +2.11.0 + + +From 6af998db05aec9af95a06f84ad94f1b96785e667 Mon Sep 17 00:00:00 2001 +From: "Daniel P. Berrange" <berrange@redhat.com> +Date: Mon, 18 Dec 2017 19:12:16 +0000 +Subject: [PATCH 02/14] ui: remove 'sync' parameter from vnc_update_client +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +There is only one caller of vnc_update_client and that always passes false +for the 'sync' parameter. + +Signed-off-by: Daniel P. Berrange <berrange@redhat.com> +Reviewed-by: Darren Kenny <darren.kenny@oracle.com> +Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> +Message-id: 20171218191228.31018-2-berrange@redhat.com +Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> +--- + ui/vnc.c | 11 +++-------- + 1 file changed, 3 insertions(+), 8 deletions(-) + +diff --git a/ui/vnc.c b/ui/vnc.c +index 7d537b5c6b..d72a61bde3 100644 +--- a/ui/vnc.c ++++ b/ui/vnc.c +@@ -596,7 +596,7 @@ VncInfo2List *qmp_query_vnc_servers(Error **errp) + 3) resolutions > 1024 + */ + +-static int vnc_update_client(VncState *vs, int has_dirty, bool sync); ++static int vnc_update_client(VncState *vs, int has_dirty); + static void vnc_disconnect_start(VncState *vs); + + static void vnc_colordepth(VncState *vs); +@@ -961,7 +961,7 @@ static int find_and_clear_dirty_height(VncState *vs, + return h; + } + +-static int vnc_update_client(VncState *vs, int has_dirty, bool sync) ++static int vnc_update_client(VncState *vs, int has_dirty) + { + if (vs->disconnecting) { + vnc_disconnect_finish(vs); +@@ -1025,9 +1025,6 @@ static int vnc_update_client(VncState *vs, int has_dirty, bool sync) + } + + vnc_job_push(job); +- if (sync) { +- vnc_jobs_join(vs); +- } + vs->force_update = 0; + vs->has_dirty = 0; + return n; +@@ -1035,8 +1032,6 @@ static int vnc_update_client(VncState *vs, int has_dirty, bool sync) + + if (vs->disconnecting) { + vnc_disconnect_finish(vs); +- } else if (sync) { +- vnc_jobs_join(vs); + } + + return 0; +@@ -2863,7 +2858,7 @@ static void vnc_refresh(DisplayChangeListener *dcl) + vnc_unlock_display(vd); + + QTAILQ_FOREACH_SAFE(vs, &vd->clients, next, vn) { +- rects += vnc_update_client(vs, has_dirty, false); ++ rects += vnc_update_client(vs, has_dirty); + /* vs might be free()ed here */ + } + +-- +2.11.0 + + +From c53df961617736f94731d94b62c2954c261d2bae Mon Sep 17 00:00:00 2001 +From: "Daniel P. Berrange" <berrange@redhat.com> +Date: Mon, 18 Dec 2017 19:12:17 +0000 +Subject: [PATCH 03/14] ui: remove unreachable code in vnc_update_client +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +A previous commit: + + commit 5a8be0f73d6f60ff08746377eb09ca459f39deab + Author: Gerd Hoffmann <kraxel@redhat.com> + Date: Wed Jul 13 12:21:20 2016 +0200 + + vnc: make sure we finish disconnect + +Added a check for vs->disconnecting at the very start of the +vnc_update_client method. This means that the very next "if" +statement check for !vs->disconnecting always evaluates true, +and is thus redundant. This in turn means the vs->disconnecting +check at the very end of the method never evaluates true, and +is thus unreachable code. + +Signed-off-by: Daniel P. Berrange <berrange@redhat.com> +Reviewed-by: Darren Kenny <darren.kenny@oracle.com> +Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> +Message-id: 20171218191228.31018-3-berrange@redhat.com +Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> +--- + ui/vnc.c | 6 +----- + 1 file changed, 1 insertion(+), 5 deletions(-) + +diff --git a/ui/vnc.c b/ui/vnc.c +index d72a61bde3..29a7208475 100644 +--- a/ui/vnc.c ++++ b/ui/vnc.c +@@ -969,7 +969,7 @@ static int vnc_update_client(VncState *vs, int has_dirty) + } + + vs->has_dirty += has_dirty; +- if (vs->need_update && !vs->disconnecting) { ++ if (vs->need_update) { + VncDisplay *vd = vs->vd; + VncJob *job; + int y; +@@ -1030,10 +1030,6 @@ static int vnc_update_client(VncState *vs, int has_dirty) + return n; + } + +- if (vs->disconnecting) { +- vnc_disconnect_finish(vs); +- } +- + return 0; + } + +-- +2.11.0 + + +From b939eb89b6f320544a9328fa908d881d0024c1ee Mon Sep 17 00:00:00 2001 +From: "Daniel P. Berrange" <berrange@redhat.com> +Date: Mon, 18 Dec 2017 19:12:18 +0000 +Subject: [PATCH 04/14] ui: remove redundant indentation in vnc_client_update +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +Now that previous dead / unreachable code has been removed, we can simplify +the indentation in the vnc_client_update method. + +Signed-off-by: Daniel P. Berrange <berrange@redhat.com> +Reviewed-by: Darren Kenny <darren.kenny@oracle.com> +Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> +Message-id: 20171218191228.31018-4-berrange@redhat.com +Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> +--- + ui/vnc.c | 112 ++++++++++++++++++++++++++++++++------------------------------- + 1 file changed, 57 insertions(+), 55 deletions(-) + +diff --git a/ui/vnc.c b/ui/vnc.c +index 29a7208475..7582111ca6 100644 +--- a/ui/vnc.c ++++ b/ui/vnc.c +@@ -963,74 +963,76 @@ static int find_and_clear_dirty_height(VncState *vs, + + static int vnc_update_client(VncState *vs, int has_dirty) + { ++ VncDisplay *vd = vs->vd; ++ VncJob *job; ++ int y; ++ int height, width; ++ int n = 0; ++ + if (vs->disconnecting) { + vnc_disconnect_finish(vs); + return 0; + } + + vs->has_dirty += has_dirty; +- if (vs->need_update) { +- VncDisplay *vd = vs->vd; +- VncJob *job; +- int y; +- int height, width; +- int n = 0; +- +- if (vs->output.offset && !vs->audio_cap && !vs->force_update) +- /* kernel send buffers are full -> drop frames to throttle */ +- return 0; ++ if (!vs->need_update) { ++ return 0; ++ } + +- if (!vs->has_dirty && !vs->audio_cap && !vs->force_update) +- return 0; ++ if (vs->output.offset && !vs->audio_cap && !vs->force_update) { ++ /* kernel send buffers are full -> drop frames to throttle */ ++ return 0; ++ } + +- /* +- * Send screen updates to the vnc client using the server +- * surface and server dirty map. guest surface updates +- * happening in parallel don't disturb us, the next pass will +- * send them to the client. +- */ +- job = vnc_job_new(vs); +- +- height = pixman_image_get_height(vd->server); +- width = pixman_image_get_width(vd->server); +- +- y = 0; +- for (;;) { +- int x, h; +- unsigned long x2; +- unsigned long offset = find_next_bit((unsigned long *) &vs->dirty, +- height * VNC_DIRTY_BPL(vs), +- y * VNC_DIRTY_BPL(vs)); +- if (offset == height * VNC_DIRTY_BPL(vs)) { +- /* no more dirty bits */ ++ if (!vs->has_dirty && !vs->audio_cap && !vs->force_update) { ++ return 0; ++ } ++ ++ /* ++ * Send screen updates to the vnc client using the server ++ * surface and server dirty map. guest surface updates ++ * happening in parallel don't disturb us, the next pass will ++ * send them to the client. ++ */ ++ job = vnc_job_new(vs); ++ ++ height = pixman_image_get_height(vd->server); ++ width = pixman_image_get_width(vd->server); ++ ++ y = 0; ++ for (;;) { ++ int x, h; ++ unsigned long x2; ++ unsigned long offset = find_next_bit((unsigned long *) &vs->dirty, ++ height * VNC_DIRTY_BPL(vs), ++ y * VNC_DIRTY_BPL(vs)); ++ if (offset == height * VNC_DIRTY_BPL(vs)) { ++ /* no more dirty bits */ ++ break; ++ } ++ y = offset / VNC_DIRTY_BPL(vs); ++ x = offset % VNC_DIRTY_BPL(vs); ++ x2 = find_next_zero_bit((unsigned long *) &vs->dirty[y], ++ VNC_DIRTY_BPL(vs), x); ++ bitmap_clear(vs->dirty[y], x, x2 - x); ++ h = find_and_clear_dirty_height(vs, y, x, x2, height); ++ x2 = MIN(x2, width / VNC_DIRTY_PIXELS_PER_BIT); ++ if (x2 > x) { ++ n += vnc_job_add_rect(job, x * VNC_DIRTY_PIXELS_PER_BIT, y, ++ (x2 - x) * VNC_DIRTY_PIXELS_PER_BIT, h); ++ } ++ if (!x && x2 == width / VNC_DIRTY_PIXELS_PER_BIT) { ++ y += h; ++ if (y == height) { + break; + } +- y = offset / VNC_DIRTY_BPL(vs); +- x = offset % VNC_DIRTY_BPL(vs); +- x2 = find_next_zero_bit((unsigned long *) &vs->dirty[y], +- VNC_DIRTY_BPL(vs), x); +- bitmap_clear(vs->dirty[y], x, x2 - x); +- h = find_and_clear_dirty_height(vs, y, x, x2, height); +- x2 = MIN(x2, width / VNC_DIRTY_PIXELS_PER_BIT); +- if (x2 > x) { +- n += vnc_job_add_rect(job, x * VNC_DIRTY_PIXELS_PER_BIT, y, +- (x2 - x) * VNC_DIRTY_PIXELS_PER_BIT, h); +- } +- if (!x && x2 == width / VNC_DIRTY_PIXELS_PER_BIT) { +- y += h; +- if (y == height) { +- break; +- } +- } + } +- +- vnc_job_push(job); +- vs->force_update = 0; +- vs->has_dirty = 0; +- return n; + } + +- return 0; ++ vnc_job_push(job); ++ vs->force_update = 0; ++ vs->has_dirty = 0; ++ return n; + } + + /* audio */ +-- +2.11.0 + + +From 3541b08475d51bddf8aded36576a0ff5a547a978 Mon Sep 17 00:00:00 2001 +From: "Daniel P. Berrange" <berrange@redhat.com> +Date: Mon, 18 Dec 2017 19:12:19 +0000 +Subject: [PATCH 05/14] ui: avoid pointless VNC updates if framebuffer isn't + dirty +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +The vnc_update_client() method checks the 'has_dirty' flag to see if there are +dirty regions that are pending to send to the client. Regardless of this flag, +if a forced update is requested, updates must be sent. For unknown reasons +though, the code also tries to sent updates if audio capture is enabled. This +makes no sense as audio capture state does not impact framebuffer contents, so +this check is removed. + +Signed-off-by: Daniel P. Berrange <berrange@redhat.com> +Reviewed-by: Darren Kenny <darren.kenny@oracle.com> +Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> +Message-id: 20171218191228.31018-5-berrange@redhat.com +Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> +--- + ui/vnc.c | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/ui/vnc.c b/ui/vnc.c +index 7582111ca6..a79848f083 100644 +--- a/ui/vnc.c ++++ b/ui/vnc.c +@@ -984,7 +984,7 @@ static int vnc_update_client(VncState *vs, int has_dirty) + return 0; + } + +- if (!vs->has_dirty && !vs->audio_cap && !vs->force_update) { ++ if (!vs->has_dirty && !vs->force_update) { + return 0; + } + +-- +2.11.0 + + +From 8f61f1c5a6bc06438a1172efa80bc7606594fa07 Mon Sep 17 00:00:00 2001 +From: "Daniel P. Berrange" <berrange@redhat.com> +Date: Mon, 18 Dec 2017 19:12:20 +0000 +Subject: [PATCH 06/14] ui: track how much decoded data we consumed when doing + SASL encoding +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +When we encode data for writing with SASL, we encode the entire pending output +buffer. The subsequent write, however, may not be able to send the full encoded +data in one go though, particularly with a slow network. So we delay setting the +output buffer offset back to zero until all the SASL encoded data is sent. + +Between encoding the data and completing sending of the SASL encoded data, +however, more data might have been placed on the pending output buffer. So it +is not valid to set offset back to zero. Instead we must keep track of how much +data we consumed during encoding and subtract only that amount. + +With the current bug we would be throwing away some pending data without having +sent it at all. By sheer luck this did not previously cause any serious problem +because appending data to the send buffer is always an atomic action, so we +only ever throw away complete RFB protocol messages. In the case of frame buffer +updates we'd catch up fairly quickly, so no obvious problem was visible. + +Signed-off-by: Daniel P. Berrange <berrange@redhat.com> +Reviewed-by: Darren Kenny <darren.kenny@oracle.com> +Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> +Message-id: 20171218191228.31018-6-berrange@redhat.com +Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> +--- + ui/vnc-auth-sasl.c | 3 ++- + ui/vnc-auth-sasl.h | 1 + + 2 files changed, 3 insertions(+), 1 deletion(-) + +diff --git a/ui/vnc-auth-sasl.c b/ui/vnc-auth-sasl.c +index 23f28280e7..761493b9b2 100644 +--- a/ui/vnc-auth-sasl.c ++++ b/ui/vnc-auth-sasl.c +@@ -67,6 +67,7 @@ long vnc_client_write_sasl(VncState *vs) + if (err != SASL_OK) + return vnc_client_io_error(vs, -1, NULL); + ++ vs->sasl.encodedRawLength = vs->output.offset; + vs->sasl.encodedOffset = 0; + } + +@@ -78,7 +79,7 @@ long vnc_client_write_sasl(VncState *vs) + + vs->sasl.encodedOffset += ret; + if (vs->sasl.encodedOffset == vs->sasl.encodedLength) { +- vs->output.offset = 0; ++ vs->output.offset -= vs->sasl.encodedRawLength; + vs->sasl.encoded = NULL; + vs->sasl.encodedOffset = vs->sasl.encodedLength = 0; + } +diff --git a/ui/vnc-auth-sasl.h b/ui/vnc-auth-sasl.h +index cb42745a6b..b9d8de1c10 100644 +--- a/ui/vnc-auth-sasl.h ++++ b/ui/vnc-auth-sasl.h +@@ -53,6 +53,7 @@ struct VncStateSASL { + */ + const uint8_t *encoded; + unsigned int encodedLength; ++ unsigned int encodedRawLength; + unsigned int encodedOffset; + char *username; + char *mechlist; +-- +2.11.0 + + +From fef1bbadfb2c3027208eb3d14b43e1bdb51166ca Mon Sep 17 00:00:00 2001 +From: "Daniel P. Berrange" <berrange@redhat.com> +Date: Mon, 18 Dec 2017 19:12:21 +0000 +Subject: [PATCH 07/14] ui: introduce enum to track VNC client framebuffer + update request state +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +Currently the VNC servers tracks whether a client has requested an incremental +or forced update with two boolean flags. There are only really 3 distinct +states to track, so create an enum to more accurately reflect permitted states. + +Signed-off-by: Daniel P. Berrange <berrange@redhat.com> +Reviewed-by: Darren Kenny <darren.kenny@oracle.com> +Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> +Message-id: 20171218191228.31018-7-berrange@redhat.com +Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> +--- + ui/vnc.c | 21 +++++++++++---------- + ui/vnc.h | 9 +++++++-- + 2 files changed, 18 insertions(+), 12 deletions(-) + +diff --git a/ui/vnc.c b/ui/vnc.c +index a79848f083..30e2feeae3 100644 +--- a/ui/vnc.c ++++ b/ui/vnc.c +@@ -975,16 +975,17 @@ static int vnc_update_client(VncState *vs, int has_dirty) + } + + vs->has_dirty += has_dirty; +- if (!vs->need_update) { ++ if (vs->update == VNC_STATE_UPDATE_NONE) { + return 0; + } + +- if (vs->output.offset && !vs->audio_cap && !vs->force_update) { ++ if (vs->output.offset && !vs->audio_cap && ++ vs->update != VNC_STATE_UPDATE_FORCE) { + /* kernel send buffers are full -> drop frames to throttle */ + return 0; + } + +- if (!vs->has_dirty && !vs->force_update) { ++ if (!vs->has_dirty && vs->update != VNC_STATE_UPDATE_FORCE) { + return 0; + } + +@@ -1030,7 +1031,7 @@ static int vnc_update_client(VncState *vs, int has_dirty) + } + + vnc_job_push(job); +- vs->force_update = 0; ++ vs->update = VNC_STATE_UPDATE_INCREMENTAL; + vs->has_dirty = 0; + return n; + } +@@ -1869,14 +1870,14 @@ static void ext_key_event(VncState *vs, int down, + static void framebuffer_update_request(VncState *vs, int incremental, + int x, int y, int w, int h) + { +- vs->need_update = 1; +- + if (incremental) { +- return; ++ if (vs->update != VNC_STATE_UPDATE_FORCE) { ++ vs->update = VNC_STATE_UPDATE_INCREMENTAL; ++ } ++ } else { ++ vs->update = VNC_STATE_UPDATE_FORCE; ++ vnc_set_area_dirty(vs->dirty, vs->vd, x, y, w, h); + } +- +- vs->force_update = 1; +- vnc_set_area_dirty(vs->dirty, vs->vd, x, y, w, h); + } + + static void send_ext_key_event_ack(VncState *vs) +diff --git a/ui/vnc.h b/ui/vnc.h +index 694cf32ca9..b9d310e640 100644 +--- a/ui/vnc.h ++++ b/ui/vnc.h +@@ -252,6 +252,12 @@ struct VncJob + QTAILQ_ENTRY(VncJob) next; + }; + ++typedef enum { ++ VNC_STATE_UPDATE_NONE, ++ VNC_STATE_UPDATE_INCREMENTAL, ++ VNC_STATE_UPDATE_FORCE, ++} VncStateUpdate; ++ + struct VncState + { + QIOChannelSocket *sioc; /* The underlying socket */ +@@ -264,8 +270,7 @@ struct VncState + * vnc-jobs-async.c */ + + VncDisplay *vd; +- int need_update; +- int force_update; ++ VncStateUpdate update; /* Most recent pending request from client */ + int has_dirty; + uint32_t features; + int absolute; +-- +2.11.0 + + +From 728a7ac95484a7ba5e624ccbac4c1326571576b0 Mon Sep 17 00:00:00 2001 +From: "Daniel P. Berrange" <berrange@redhat.com> +Date: Mon, 18 Dec 2017 19:12:22 +0000 +Subject: [PATCH 08/14] ui: correctly reset framebuffer update state after + processing dirty regions +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +According to the RFB protocol, a client sends one or more framebuffer update +requests to the server. The server can reply with a single framebuffer update +response, that covers all previously received requests. Once the client has +read this update from the server, it may send further framebuffer update +requests to monitor future changes. The client is free to delay sending the +framebuffer update request if it needs to throttle the amount of data it is +reading from the server. + +The QEMU VNC server, however, has never correctly handled the framebuffer +update requests. Once QEMU has received an update request, it will continue to +send client updates forever, even if the client hasn't asked for further +updates. This prevents the client from throttling back data it gets from the +server. This change fixes the flawed logic such that after a set of updates are +sent out, QEMU waits for a further update request before sending more data. + +Signed-off-by: Daniel P. Berrange <berrange@redhat.com> +Reviewed-by: Darren Kenny <darren.kenny@oracle.com> +Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> +Message-id: 20171218191228.31018-8-berrange@redhat.com +Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> +--- + ui/vnc.c | 2 +- + 1 file changed, 1 insertion(+), 1 deletion(-) + +diff --git a/ui/vnc.c b/ui/vnc.c +index 30e2feeae3..243c72be13 100644 +--- a/ui/vnc.c ++++ b/ui/vnc.c +@@ -1031,7 +1031,7 @@ static int vnc_update_client(VncState *vs, int has_dirty) + } + + vnc_job_push(job); +- vs->update = VNC_STATE_UPDATE_INCREMENTAL; ++ vs->update = VNC_STATE_UPDATE_NONE; + vs->has_dirty = 0; + return n; + } +-- +2.11.0 + + +From 0bad834228b9ee63e4239108d02dcb94568254d0 Mon Sep 17 00:00:00 2001 +From: "Daniel P. Berrange" <berrange@redhat.com> +Date: Mon, 18 Dec 2017 19:12:23 +0000 +Subject: [PATCH 09/14] ui: refactor code for determining if an update should + be sent to the client +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +The logic for determining if it is possible to send an update to the client +will become more complicated shortly, so pull it out into a separate method +for easier extension later. + +Signed-off-by: Daniel P. Berrange <berrange@redhat.com> +Reviewed-by: Darren Kenny <darren.kenny@oracle.com> +Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> +Message-id: 20171218191228.31018-9-berrange@redhat.com +Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> +--- + ui/vnc.c | 27 ++++++++++++++++++++------- + 1 file changed, 20 insertions(+), 7 deletions(-) + +diff --git a/ui/vnc.c b/ui/vnc.c +index 243c72be13..4ba7fc076a 100644 +--- a/ui/vnc.c ++++ b/ui/vnc.c +@@ -961,6 +961,25 @@ static int find_and_clear_dirty_height(VncState *vs, + return h; + } + ++static bool vnc_should_update(VncState *vs) ++{ ++ switch (vs->update) { ++ case VNC_STATE_UPDATE_NONE: ++ break; ++ case VNC_STATE_UPDATE_INCREMENTAL: ++ /* Only allow incremental updates if the output buffer ++ * is empty, or if audio capture is enabled. ++ */ ++ if (!vs->output.offset || vs->audio_cap) { ++ return true; ++ } ++ break; ++ case VNC_STATE_UPDATE_FORCE: ++ return true; ++ } ++ return false; ++} ++ + static int vnc_update_client(VncState *vs, int has_dirty) + { + VncDisplay *vd = vs->vd; +@@ -975,13 +994,7 @@ static int vnc_update_client(VncState *vs, int has_dirty) + } + + vs->has_dirty += has_dirty; +- if (vs->update == VNC_STATE_UPDATE_NONE) { +- return 0; +- } +- +- if (vs->output.offset && !vs->audio_cap && +- vs->update != VNC_STATE_UPDATE_FORCE) { +- /* kernel send buffers are full -> drop frames to throttle */ ++ if (!vnc_should_update(vs)) { + return 0; + } + +-- +2.11.0 + + +From e2b72cb6e0443d90d7ab037858cb6834b6cca852 Mon Sep 17 00:00:00 2001 +From: "Daniel P. Berrange" <berrange@redhat.com> +Date: Mon, 18 Dec 2017 19:12:24 +0000 +Subject: [PATCH 10/14] ui: fix VNC client throttling when audio capture is + active +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +The VNC server must throttle data sent to the client to prevent the 'output' +buffer size growing without bound, if the client stops reading data off the +socket (either maliciously or due to stalled/slow network connection). + +The current throttling is very crude because it simply checks whether the +output buffer offset is zero. This check must be disabled if audio capture is +enabled, because when streaming audio the output buffer offset will rarely be +zero due to queued audio data, and so this would starve framebuffer updates. + +As a result, the VNC client can cause QEMU to allocate arbitrary amounts of RAM. +They can first start something in the guest that triggers lots of framebuffer +updates eg play a youtube video. Then enable audio capture, and simply never +read data back from the server. This can easily make QEMU's VNC server send +buffer consume 100MB of RAM per second, until the OOM killer starts reaping +processes (hopefully the rogue QEMU process, but it might pick others...). + +To address this we make the throttling more intelligent, so we can throttle +when audio capture is active too. To determine how to throttle incremental +updates or audio data, we calculate a size threshold. Normally the threshold is +the approximate number of bytes associated with a single complete framebuffer +update. ie width * height * bytes per pixel. We'll send incremental updates +until we hit this threshold, at which point we'll stop sending updates until +data has been written to the wire, causing the output buffer offset to fall +back below the threshold. + +If audio capture is enabled, we increase the size of the threshold to also +allow for upto 1 seconds worth of audio data samples. ie nchannels * bytes +per sample * frequency. This allows the output buffer to have a mixture of +incremental framebuffer updates and audio data queued, but once the threshold +is exceeded, audio data will be dropped and incremental updates will be +throttled. + +This unbounded memory growth affects all VNC server configurations supported by +QEMU, with no workaround possible. The mitigating factor is that it can only be +triggered by a client that has authenticated with the VNC server, and who is +able to trigger a large quantity of framebuffer updates or audio samples from +the guest OS. Mostly they'll just succeed in getting the OOM killer to kill +their own QEMU process, but its possible other processes can get taken out as +collateral damage. + +This is a more general variant of the similar unbounded memory usage flaw in +the websockets server, that was previously assigned CVE-2017-15268, and fixed +in 2.11 by: + + commit a7b20a8efa28e5f22c26c06cd06c2f12bc863493 + Author: Daniel P. Berrange <berrange@redhat.com> + Date: Mon Oct 9 14:43:42 2017 +0100 + + io: monitor encoutput buffer size from websocket GSource + +This new general memory usage flaw has been assigned CVE-2017-15124, and is +partially fixed by this patch. + +Signed-off-by: Daniel P. Berrange <berrange@redhat.com> +Reviewed-by: Darren Kenny <darren.kenny@oracle.com> +Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> +Message-id: 20171218191228.31018-10-berrange@redhat.com +Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> +--- + ui/vnc.c | 72 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++-------- + ui/vnc.h | 6 ++++++ + 2 files changed, 70 insertions(+), 8 deletions(-) + +diff --git a/ui/vnc.c b/ui/vnc.c +index 4ba7fc076a..9e03cc7c01 100644 +--- a/ui/vnc.c ++++ b/ui/vnc.c +@@ -60,6 +60,7 @@ static QTAILQ_HEAD(, VncDisplay) vnc_displays = + + static int vnc_cursor_define(VncState *vs); + static void vnc_release_modifiers(VncState *vs); ++static void vnc_update_throttle_offset(VncState *vs); + + static void vnc_set_share_mode(VncState *vs, VncShareMode mode) + { +@@ -766,6 +767,7 @@ static void vnc_dpy_switch(DisplayChangeListener *dcl, + vnc_set_area_dirty(vs->dirty, vd, 0, 0, + vnc_width(vd), + vnc_height(vd)); ++ vnc_update_throttle_offset(vs); + } + } + +@@ -961,16 +963,67 @@ static int find_and_clear_dirty_height(VncState *vs, + return h; + } + ++/* ++ * Figure out how much pending data we should allow in the output ++ * buffer before we throttle incremental display updates, and/or ++ * drop audio samples. ++ * ++ * We allow for equiv of 1 full display's worth of FB updates, ++ * and 1 second of audio samples. If audio backlog was larger ++ * than that the client would already suffering awful audio ++ * glitches, so dropping samples is no worse really). ++ */ ++static void vnc_update_throttle_offset(VncState *vs) ++{ ++ size_t offset = ++ vs->client_width * vs->client_height * vs->client_pf.bytes_per_pixel; ++ ++ if (vs->audio_cap) { ++ int freq = vs->as.freq; ++ /* We don't limit freq when reading settings from client, so ++ * it could be upto MAX_INT in size. 48khz is a sensible ++ * upper bound for trustworthy clients */ ++ int bps; ++ if (freq > 48000) { ++ freq = 48000; ++ } ++ switch (vs->as.fmt) { ++ default: ++ case AUD_FMT_U8: ++ case AUD_FMT_S8: ++ bps = 1; ++ break; ++ case AUD_FMT_U16: ++ case AUD_FMT_S16: ++ bps = 2; ++ break; ++ case AUD_FMT_U32: ++ case AUD_FMT_S32: ++ bps = 4; ++ break; ++ } ++ offset += freq * bps * vs->as.nchannels; ++ } ++ ++ /* Put a floor of 1MB on offset, so that if we have a large pending ++ * buffer and the display is resized to a small size & back again ++ * we don't suddenly apply a tiny send limit ++ */ ++ offset = MAX(offset, 1024 * 1024); ++ ++ vs->throttle_output_offset = offset; ++} ++ + static bool vnc_should_update(VncState *vs) + { + switch (vs->update) { + case VNC_STATE_UPDATE_NONE: + break; + case VNC_STATE_UPDATE_INCREMENTAL: +- /* Only allow incremental updates if the output buffer +- * is empty, or if audio capture is enabled. ++ /* Only allow incremental updates if the pending send queue ++ * is less than the permitted threshold + */ +- if (!vs->output.offset || vs->audio_cap) { ++ if (vs->output.offset < vs->throttle_output_offset) { + return true; + } + break; +@@ -1084,11 +1137,13 @@ static void audio_capture(void *opaque, void *buf, int size) + VncState *vs = opaque; + + vnc_lock_output(vs); +- vnc_write_u8(vs, VNC_MSG_SERVER_QEMU); +- vnc_write_u8(vs, VNC_MSG_SERVER_QEMU_AUDIO); +- vnc_write_u16(vs, VNC_MSG_SERVER_QEMU_AUDIO_DATA); +- vnc_write_u32(vs, size); +- vnc_write(vs, buf, size); ++ if (vs->output.offset < vs->throttle_output_offset) { ++ vnc_write_u8(vs, VNC_MSG_SERVER_QEMU); ++ vnc_write_u8(vs, VNC_MSG_SERVER_QEMU_AUDIO); ++ vnc_write_u16(vs, VNC_MSG_SERVER_QEMU_AUDIO_DATA); ++ vnc_write_u32(vs, size); ++ vnc_write(vs, buf, size); ++ } + vnc_unlock_output(vs); + vnc_flush(vs); + } +@@ -2288,6 +2343,7 @@ static int protocol_client_msg(VncState *vs, uint8_t *data, size_t len) + break; + } + ++ vnc_update_throttle_offset(vs); + vnc_read_when(vs, protocol_client_msg, 1); + return 0; + } +diff --git a/ui/vnc.h b/ui/vnc.h +index b9d310e640..8fe69595c6 100644 +--- a/ui/vnc.h ++++ b/ui/vnc.h +@@ -298,6 +298,12 @@ struct VncState + + VncClientInfo *info; + ++ /* We allow multiple incremental updates or audio capture ++ * samples to be queued in output buffer, provided the ++ * buffer size doesn't exceed this threshold. The value ++ * is calculating dynamically based on framebuffer size ++ * and audio sample settings in vnc_update_throttle_offset() */ ++ size_t throttle_output_offset; + Buffer output; + Buffer input; + /* current output mode information */ +-- +2.11.0 + + +From ada8d2e4369ea49677d8672ac81bce73eefd5b54 Mon Sep 17 00:00:00 2001 +From: "Daniel P. Berrange" <berrange@redhat.com> +Date: Mon, 18 Dec 2017 19:12:25 +0000 +Subject: [PATCH 11/14] ui: fix VNC client throttling when forced update is + requested +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +The VNC server must throttle data sent to the client to prevent the 'output' +buffer size growing without bound, if the client stops reading data off the +socket (either maliciously or due to stalled/slow network connection). + +The current throttling is very crude because it simply checks whether the +output buffer offset is zero. This check is disabled if the client has requested +a forced update, because we want to send these as soon as possible. + +As a result, the VNC client can cause QEMU to allocate arbitrary amounts of RAM. +They can first start something in the guest that triggers lots of framebuffer +updates eg play a youtube video. Then repeatedly send full framebuffer update +requests, but never read data back from the server. This can easily make QEMU's +VNC server send buffer consume 100MB of RAM per second, until the OOM killer +starts reaping processes (hopefully the rogue QEMU process, but it might pick +others...). + +To address this we make the throttling more intelligent, so we can throttle +full updates. When we get a forced update request, we keep track of exactly how +much data we put on the output buffer. We will not process a subsequent forced +update request until this data has been fully sent on the wire. We always allow +one forced update request to be in flight, regardless of what data is queued +for incremental updates or audio data. The slight complication is that we do +not initially know how much data an update will send, as this is done in the +background by the VNC job thread. So we must track the fact that the job thread +has an update pending, and not process any further updates until this job is +has been completed & put data on the output buffer. + +This unbounded memory growth affects all VNC server configurations supported by +QEMU, with no workaround possible. The mitigating factor is that it can only be +triggered by a client that has authenticated with the VNC server, and who is +able to trigger a large quantity of framebuffer updates or audio samples from +the guest OS. Mostly they'll just succeed in getting the OOM killer to kill +their own QEMU process, but its possible other processes can get taken out as +collateral damage. + +This is a more general variant of the similar unbounded memory usage flaw in +the websockets server, that was previously assigned CVE-2017-15268, and fixed +in 2.11 by: + + commit a7b20a8efa28e5f22c26c06cd06c2f12bc863493 + Author: Daniel P. Berrange <berrange@redhat.com> + Date: Mon Oct 9 14:43:42 2017 +0100 + + io: monitor encoutput buffer size from websocket GSource + +This new general memory usage flaw has been assigned CVE-2017-15124, and is +partially fixed by this patch. + +Signed-off-by: Daniel P. Berrange <berrange@redhat.com> +Reviewed-by: Darren Kenny <darren.kenny@oracle.com> +Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> +Message-id: 20171218191228.31018-11-berrange@redhat.com +Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> +--- + ui/vnc-auth-sasl.c | 5 +++++ + ui/vnc-jobs.c | 5 +++++ + ui/vnc.c | 28 ++++++++++++++++++++++++---- + ui/vnc.h | 7 +++++++ + 4 files changed, 41 insertions(+), 4 deletions(-) + +diff --git a/ui/vnc-auth-sasl.c b/ui/vnc-auth-sasl.c +index 761493b9b2..8c1cdde3db 100644 +--- a/ui/vnc-auth-sasl.c ++++ b/ui/vnc-auth-sasl.c +@@ -79,6 +79,11 @@ long vnc_client_write_sasl(VncState *vs) + + vs->sasl.encodedOffset += ret; + if (vs->sasl.encodedOffset == vs->sasl.encodedLength) { ++ if (vs->sasl.encodedRawLength >= vs->force_update_offset) { ++ vs->force_update_offset = 0; ++ } else { ++ vs->force_update_offset -= vs->sasl.encodedRawLength; ++ } + vs->output.offset -= vs->sasl.encodedRawLength; + vs->sasl.encoded = NULL; + vs->sasl.encodedOffset = vs->sasl.encodedLength = 0; +diff --git a/ui/vnc-jobs.c b/ui/vnc-jobs.c +index f7867771ae..e326679dd0 100644 +--- a/ui/vnc-jobs.c ++++ b/ui/vnc-jobs.c +@@ -152,6 +152,11 @@ void vnc_jobs_consume_buffer(VncState *vs) + vs->ioc, G_IO_IN | G_IO_OUT, vnc_client_io, vs, NULL); + } + buffer_move(&vs->output, &vs->jobs_buffer); ++ ++ if (vs->job_update == VNC_STATE_UPDATE_FORCE) { ++ vs->force_update_offset = vs->output.offset; ++ } ++ vs->job_update = VNC_STATE_UPDATE_NONE; + } + flush = vs->ioc != NULL && vs->abort != true; + vnc_unlock_output(vs); +diff --git a/ui/vnc.c b/ui/vnc.c +index 9e03cc7c01..4805ac41d0 100644 +--- a/ui/vnc.c ++++ b/ui/vnc.c +@@ -1021,14 +1021,28 @@ static bool vnc_should_update(VncState *vs) + break; + case VNC_STATE_UPDATE_INCREMENTAL: + /* Only allow incremental updates if the pending send queue +- * is less than the permitted threshold ++ * is less than the permitted threshold, and the job worker ++ * is completely idle. + */ +- if (vs->output.offset < vs->throttle_output_offset) { ++ if (vs->output.offset < vs->throttle_output_offset && ++ vs->job_update == VNC_STATE_UPDATE_NONE) { + return true; + } + break; + case VNC_STATE_UPDATE_FORCE: +- return true; ++ /* Only allow forced updates if the pending send queue ++ * does not contain a previous forced update, and the ++ * job worker is completely idle. ++ * ++ * Note this means we'll queue a forced update, even if ++ * the output buffer size is otherwise over the throttle ++ * output limit. ++ */ ++ if (vs->force_update_offset == 0 && ++ vs->job_update == VNC_STATE_UPDATE_NONE) { ++ return true; ++ } ++ break; + } + return false; + } +@@ -1096,8 +1110,9 @@ static int vnc_update_client(VncState *vs, int has_dirty) + } + } + +- vnc_job_push(job); ++ vs->job_update = vs->update; + vs->update = VNC_STATE_UPDATE_NONE; ++ vnc_job_push(job); + vs->has_dirty = 0; + return n; + } +@@ -1332,6 +1347,11 @@ static ssize_t vnc_client_write_plain(VncState *vs) + if (!ret) + return 0; + ++ if (ret >= vs->force_update_offset) { ++ vs->force_update_offset = 0; ++ } else { ++ vs->force_update_offset -= ret; ++ } + buffer_advance(&vs->output, ret); + + if (vs->output.offset == 0) { +diff --git a/ui/vnc.h b/ui/vnc.h +index 8fe69595c6..3f4cd4d93d 100644 +--- a/ui/vnc.h ++++ b/ui/vnc.h +@@ -271,6 +271,7 @@ struct VncState + + VncDisplay *vd; + VncStateUpdate update; /* Most recent pending request from client */ ++ VncStateUpdate job_update; /* Currently processed by job thread */ + int has_dirty; + uint32_t features; + int absolute; +@@ -298,6 +299,12 @@ struct VncState + + VncClientInfo *info; + ++ /* Job thread bottom half has put data for a forced update ++ * into the output buffer. This offset points to the end of ++ * the update data in the output buffer. This lets us determine ++ * when a force update is fully sent to the client, allowing ++ * us to process further forced updates. */ ++ size_t force_update_offset; + /* We allow multiple incremental updates or audio capture + * samples to be queued in output buffer, provided the + * buffer size doesn't exceed this threshold. The value +-- +2.11.0 + + +From f887cf165db20f405cb8805c716bd363aaadf815 Mon Sep 17 00:00:00 2001 +From: "Daniel P. Berrange" <berrange@redhat.com> +Date: Mon, 18 Dec 2017 19:12:26 +0000 +Subject: [PATCH 12/14] ui: place a hard cap on VNC server output buffer size +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +The previous patches fix problems with throttling of forced framebuffer updates +and audio data capture that would cause the QEMU output buffer size to grow +without bound. Those fixes are graceful in that once the client catches up with +reading data from the server, everything continues operating normally. + +There is some data which the server sends to the client that is impractical to +throttle. Specifically there are various pseudo framebuffer update encodings to +inform the client of things like desktop resizes, pointer changes, audio +playback start/stop, LED state and so on. These generally only involve sending +a very small amount of data to the client, but a malicious guest might be able +to do things that trigger these changes at a very high rate. Throttling them is +not practical as missed or delayed events would cause broken behaviour for the +client. + +This patch thus takes a more forceful approach of setting an absolute upper +bound on the amount of data we permit to be present in the output buffer at +any time. The previous patch set a threshold for throttling the output buffer +by allowing an amount of data equivalent to one complete framebuffer update and +one seconds worth of audio data. On top of this it allowed for one further +forced framebuffer update to be queued. + +To be conservative, we thus take that throttling threshold and multiply it by +5 to form an absolute upper bound. If this bound is hit during vnc_write() we +forceably disconnect the client, refusing to queue further data. This limit is +high enough that it should never be hit unless a malicious client is trying to +exploit the sever, or the network is completely saturated preventing any sending +of data on the socket. + +This completes the fix for CVE-2017-15124 started in the previous patches. + +Signed-off-by: Daniel P. Berrange <berrange@redhat.com> +Reviewed-by: Darren Kenny <darren.kenny@oracle.com> +Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> +Message-id: 20171218191228.31018-12-berrange@redhat.com +Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> +--- + ui/vnc.c | 29 +++++++++++++++++++++++++++++ + 1 file changed, 29 insertions(+) + +diff --git a/ui/vnc.c b/ui/vnc.c +index 4805ac41d0..e53e84587a 100644 +--- a/ui/vnc.c ++++ b/ui/vnc.c +@@ -1521,8 +1521,37 @@ gboolean vnc_client_io(QIOChannel *ioc G_GNUC_UNUSED, + } + + ++/* ++ * Scale factor to apply to vs->throttle_output_offset when checking for ++ * hard limit. Worst case normal usage could be x2, if we have a complete ++ * incremental update and complete forced update in the output buffer. ++ * So x3 should be good enough, but we pick x5 to be conservative and thus ++ * (hopefully) never trigger incorrectly. ++ */ ++#define VNC_THROTTLE_OUTPUT_LIMIT_SCALE 5 ++ + void vnc_write(VncState *vs, const void *data, size_t len) + { ++ if (vs->disconnecting) { ++ return; ++ } ++ /* Protection against malicious client/guest to prevent our output ++ * buffer growing without bound if client stops reading data. This ++ * should rarely trigger, because we have earlier throttling code ++ * which stops issuing framebuffer updates and drops audio data ++ * if the throttle_output_offset value is exceeded. So we only reach ++ * this higher level if a huge number of pseudo-encodings get ++ * triggered while data can't be sent on the socket. ++ * ++ * NB throttle_output_offset can be zero during early protocol ++ * handshake, or from the job thread's VncState clone ++ */ ++ if (vs->throttle_output_offset != 0 && ++ vs->output.offset > (vs->throttle_output_offset * ++ VNC_THROTTLE_OUTPUT_LIMIT_SCALE)) { ++ vnc_disconnect_start(vs); ++ return; ++ } + buffer_reserve(&vs->output, len); + + if (vs->ioc != NULL && buffer_empty(&vs->output)) { +-- +2.11.0 + + +From 6aa22a29187e1908f5db738d27c64a9efc8d0bfa Mon Sep 17 00:00:00 2001 +From: "Daniel P. Berrange" <berrange@redhat.com> +Date: Mon, 18 Dec 2017 19:12:27 +0000 +Subject: [PATCH 13/14] ui: add trace events related to VNC client throttling +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +The VNC client throttling is quite subtle so will benefit from having trace +points available for live debugging. + +Signed-off-by: Daniel P. Berrange <berrange@redhat.com> +Reviewed-by: Darren Kenny <darren.kenny@oracle.com> +Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> +Message-id: 20171218191228.31018-13-berrange@redhat.com +Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> +--- + ui/trace-events | 7 +++++++ + ui/vnc.c | 23 +++++++++++++++++++++++ + 2 files changed, 30 insertions(+) + +diff --git a/ui/trace-events b/ui/trace-events +index 1a9f126330..85f74f948b 100644 +--- a/ui/trace-events ++++ b/ui/trace-events +@@ -35,6 +35,13 @@ vnc_client_connect(void *state, void *ioc) "VNC client connect state=%p ioc=%p" + vnc_client_disconnect_start(void *state, void *ioc) "VNC client disconnect start state=%p ioc=%p" + vnc_client_disconnect_finish(void *state, void *ioc) "VNC client disconnect finish state=%p ioc=%p" + vnc_client_io_wrap(void *state, void *ioc, const char *type) "VNC client I/O wrap state=%p ioc=%p type=%s" ++vnc_client_throttle_threshold(void *state, void *ioc, size_t oldoffset, size_t offset, int client_width, int client_height, int bytes_per_pixel, void *audio_cap) "VNC client throttle threshold state=%p ioc=%p oldoffset=%zu newoffset=%zu width=%d height=%d bpp=%d audio=%p" ++vnc_client_throttle_incremental(void *state, void *ioc, int job_update, size_t offset) "VNC client throttle incremental state=%p ioc=%p job-update=%d offset=%zu" ++vnc_client_throttle_forced(void *state, void *ioc, int job_update, size_t offset) "VNC client throttle forced state=%p ioc=%p job-update=%d offset=%zu" ++vnc_client_throttle_audio(void *state, void *ioc, size_t offset) "VNC client throttle audio state=%p ioc=%p offset=%zu" ++vnc_client_unthrottle_forced(void *state, void *ioc) "VNC client unthrottle forced offset state=%p ioc=%p" ++vnc_client_unthrottle_incremental(void *state, void *ioc, size_t offset) "VNC client unthrottle incremental state=%p ioc=%p offset=%zu" ++vnc_client_output_limit(void *state, void *ioc, size_t offset, size_t threshold) "VNC client output limit state=%p ioc=%p offset=%zu threshold=%zu" + vnc_auth_init(void *display, int websock, int auth, int subauth) "VNC auth init state=%p websock=%d auth=%d subauth=%d" + vnc_auth_start(void *state, int method) "VNC client auth start state=%p method=%d" + vnc_auth_pass(void *state, int method) "VNC client auth passed state=%p method=%d" +diff --git a/ui/vnc.c b/ui/vnc.c +index e53e84587a..0a5e629d5d 100644 +--- a/ui/vnc.c ++++ b/ui/vnc.c +@@ -1011,6 +1011,12 @@ static void vnc_update_throttle_offset(VncState *vs) + */ + offset = MAX(offset, 1024 * 1024); + ++ if (vs->throttle_output_offset != offset) { ++ trace_vnc_client_throttle_threshold( ++ vs, vs->ioc, vs->throttle_output_offset, offset, vs->client_width, ++ vs->client_height, vs->client_pf.bytes_per_pixel, vs->audio_cap); ++ } ++ + vs->throttle_output_offset = offset; + } + +@@ -1028,6 +1034,8 @@ static bool vnc_should_update(VncState *vs) + vs->job_update == VNC_STATE_UPDATE_NONE) { + return true; + } ++ trace_vnc_client_throttle_incremental( ++ vs, vs->ioc, vs->job_update, vs->output.offset); + break; + case VNC_STATE_UPDATE_FORCE: + /* Only allow forced updates if the pending send queue +@@ -1042,6 +1050,8 @@ static bool vnc_should_update(VncState *vs) + vs->job_update == VNC_STATE_UPDATE_NONE) { + return true; + } ++ trace_vnc_client_throttle_forced( ++ vs, vs->ioc, vs->job_update, vs->force_update_offset); + break; + } + return false; +@@ -1158,6 +1168,8 @@ static void audio_capture(void *opaque, void *buf, int size) + vnc_write_u16(vs, VNC_MSG_SERVER_QEMU_AUDIO_DATA); + vnc_write_u32(vs, size); + vnc_write(vs, buf, size); ++ } else { ++ trace_vnc_client_throttle_audio(vs, vs->ioc, vs->output.offset); + } + vnc_unlock_output(vs); + vnc_flush(vs); +@@ -1328,6 +1340,7 @@ ssize_t vnc_client_write_buf(VncState *vs, const uint8_t *data, size_t datalen) + */ + static ssize_t vnc_client_write_plain(VncState *vs) + { ++ size_t offset; + ssize_t ret; + + #ifdef CONFIG_VNC_SASL +@@ -1348,11 +1361,19 @@ static ssize_t vnc_client_write_plain(VncState *vs) + return 0; + + if (ret >= vs->force_update_offset) { ++ if (vs->force_update_offset != 0) { ++ trace_vnc_client_unthrottle_forced(vs, vs->ioc); ++ } + vs->force_update_offset = 0; + } else { + vs->force_update_offset -= ret; + } ++ offset = vs->output.offset; + buffer_advance(&vs->output, ret); ++ if (offset >= vs->throttle_output_offset && ++ vs->output.offset < vs->throttle_output_offset) { ++ trace_vnc_client_unthrottle_incremental(vs, vs->ioc, vs->output.offset); ++ } + + if (vs->output.offset == 0) { + if (vs->ioc_tag) { +@@ -1549,6 +1570,8 @@ void vnc_write(VncState *vs, const void *data, size_t len) + if (vs->throttle_output_offset != 0 && + vs->output.offset > (vs->throttle_output_offset * + VNC_THROTTLE_OUTPUT_LIMIT_SCALE)) { ++ trace_vnc_client_output_limit(vs, vs->ioc, vs->output.offset, ++ vs->throttle_output_offset); + vnc_disconnect_start(vs); + return; + } +-- +2.11.0 + + +From 30b80fd5269257f55203b7072c505b4ebaab5115 Mon Sep 17 00:00:00 2001 +From: "Daniel P. Berrange" <berrange@redhat.com> +Date: Mon, 18 Dec 2017 19:12:28 +0000 +Subject: [PATCH 14/14] ui: mix misleading comments & return types of VNC I/O + helper methods +MIME-Version: 1.0 +Content-Type: text/plain; charset=UTF-8 +Content-Transfer-Encoding: 8bit + +While the QIOChannel APIs for reading/writing data return ssize_t, with negative +value indicating an error, the VNC code passes this return value through the +vnc_client_io_error() method. This detects the error condition, disconnects the +client and returns 0 to indicate error. Thus all the VNC helper methods should +return size_t (unsigned), and misleading comments which refer to the possibility +of negative return values need fixing. + +Signed-off-by: Daniel P. Berrange <berrange@redhat.com> +Reviewed-by: Darren Kenny <darren.kenny@oracle.com> +Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com> +Message-id: 20171218191228.31018-14-berrange@redhat.com +Signed-off-by: Gerd Hoffmann <kraxel@redhat.com> +--- + ui/vnc-auth-sasl.c | 8 ++++---- + ui/vnc-auth-sasl.h | 4 ++-- + ui/vnc.c | 29 +++++++++++++++-------------- + ui/vnc.h | 6 +++--- + 4 files changed, 24 insertions(+), 23 deletions(-) + +diff --git a/ui/vnc-auth-sasl.c b/ui/vnc-auth-sasl.c +index 8c1cdde3db..74a5f513f2 100644 +--- a/ui/vnc-auth-sasl.c ++++ b/ui/vnc-auth-sasl.c +@@ -48,9 +48,9 @@ void vnc_sasl_client_cleanup(VncState *vs) + } + + +-long vnc_client_write_sasl(VncState *vs) ++size_t vnc_client_write_sasl(VncState *vs) + { +- long ret; ++ size_t ret; + + VNC_DEBUG("Write SASL: Pending output %p size %zd offset %zd " + "Encoded: %p size %d offset %d\n", +@@ -106,9 +106,9 @@ long vnc_client_write_sasl(VncState *vs) + } + + +-long vnc_client_read_sasl(VncState *vs) ++size_t vnc_client_read_sasl(VncState *vs) + { +- long ret; ++ size_t ret; + uint8_t encoded[4096]; + const char *decoded; + unsigned int decodedLen; +diff --git a/ui/vnc-auth-sasl.h b/ui/vnc-auth-sasl.h +index b9d8de1c10..2ae224ee3a 100644 +--- a/ui/vnc-auth-sasl.h ++++ b/ui/vnc-auth-sasl.h +@@ -65,8 +65,8 @@ struct VncDisplaySASL { + + void vnc_sasl_client_cleanup(VncState *vs); + +-long vnc_client_read_sasl(VncState *vs); +-long vnc_client_write_sasl(VncState *vs); ++size_t vnc_client_read_sasl(VncState *vs); ++size_t vnc_client_write_sasl(VncState *vs); + + void start_auth_sasl(VncState *vs); + +diff --git a/ui/vnc.c b/ui/vnc.c +index 0a5e629d5d..665a143578 100644 +--- a/ui/vnc.c ++++ b/ui/vnc.c +@@ -1272,7 +1272,7 @@ void vnc_disconnect_finish(VncState *vs) + g_free(vs); + } + +-ssize_t vnc_client_io_error(VncState *vs, ssize_t ret, Error **errp) ++size_t vnc_client_io_error(VncState *vs, ssize_t ret, Error **errp) + { + if (ret <= 0) { + if (ret == 0) { +@@ -1315,9 +1315,9 @@ void vnc_client_error(VncState *vs) + * + * Returns the number of bytes written, which may be less than + * the requested 'datalen' if the socket would block. Returns +- * -1 on error, and disconnects the client socket. ++ * 0 on I/O error, and disconnects the client socket. + */ +-ssize_t vnc_client_write_buf(VncState *vs, const uint8_t *data, size_t datalen) ++size_t vnc_client_write_buf(VncState *vs, const uint8_t *data, size_t datalen) + { + Error *err = NULL; + ssize_t ret; +@@ -1335,13 +1335,13 @@ ssize_t vnc_client_write_buf(VncState *vs, const uint8_t *data, size_t datalen) + * will switch the FD poll() handler back to read monitoring. + * + * Returns the number of bytes written, which may be less than +- * the buffered output data if the socket would block. Returns +- * -1 on error, and disconnects the client socket. ++ * the buffered output data if the socket would block. Returns ++ * 0 on I/O error, and disconnects the client socket. + */ +-static ssize_t vnc_client_write_plain(VncState *vs) ++static size_t vnc_client_write_plain(VncState *vs) + { + size_t offset; +- ssize_t ret; ++ size_t ret; + + #ifdef CONFIG_VNC_SASL + VNC_DEBUG("Write Plain: Pending output %p size %zd offset %zd. Wait SSF %d\n", +@@ -1442,9 +1442,9 @@ void vnc_read_when(VncState *vs, VncReadEvent *func, size_t expecting) + * + * Returns the number of bytes read, which may be less than + * the requested 'datalen' if the socket would block. Returns +- * -1 on error, and disconnects the client socket. ++ * 0 on I/O error or EOF, and disconnects the client socket. + */ +-ssize_t vnc_client_read_buf(VncState *vs, uint8_t *data, size_t datalen) ++size_t vnc_client_read_buf(VncState *vs, uint8_t *data, size_t datalen) + { + ssize_t ret; + Error *err = NULL; +@@ -1460,12 +1460,13 @@ ssize_t vnc_client_read_buf(VncState *vs, uint8_t *data, size_t datalen) + * when not using any SASL SSF encryption layers. Will read as much + * data as possible without blocking. + * +- * Returns the number of bytes read. Returns -1 on error, and +- * disconnects the client socket. ++ * Returns the number of bytes read, which may be less than ++ * the requested 'datalen' if the socket would block. Returns ++ * 0 on I/O error or EOF, and disconnects the client socket. + */ +-static ssize_t vnc_client_read_plain(VncState *vs) ++static size_t vnc_client_read_plain(VncState *vs) + { +- ssize_t ret; ++ size_t ret; + VNC_DEBUG("Read plain %p size %zd offset %zd\n", + vs->input.buffer, vs->input.capacity, vs->input.offset); + buffer_reserve(&vs->input, 4096); +@@ -1491,7 +1492,7 @@ static void vnc_jobs_bh(void *opaque) + */ + static int vnc_client_read(VncState *vs) + { +- ssize_t ret; ++ size_t ret; + + #ifdef CONFIG_VNC_SASL + if (vs->sasl.conn && vs->sasl.runSSF) +diff --git a/ui/vnc.h b/ui/vnc.h +index 3f4cd4d93d..0c33a5f7fe 100644 +--- a/ui/vnc.h ++++ b/ui/vnc.h +@@ -524,8 +524,8 @@ gboolean vnc_client_io(QIOChannel *ioc, + GIOCondition condition, + void *opaque); + +-ssize_t vnc_client_read_buf(VncState *vs, uint8_t *data, size_t datalen); +-ssize_t vnc_client_write_buf(VncState *vs, const uint8_t *data, size_t datalen); ++size_t vnc_client_read_buf(VncState *vs, uint8_t *data, size_t datalen); ++size_t vnc_client_write_buf(VncState *vs, const uint8_t *data, size_t datalen); + + /* Protocol I/O functions */ + void vnc_write(VncState *vs, const void *data, size_t len); +@@ -544,7 +544,7 @@ uint32_t read_u32(uint8_t *data, size_t offset); + + /* Protocol stage functions */ + void vnc_client_error(VncState *vs); +-ssize_t vnc_client_io_error(VncState *vs, ssize_t ret, Error **errp); ++size_t vnc_client_io_error(VncState *vs, ssize_t ret, Error **errp); + + void start_client_init(VncState *vs); + void start_auth_vnc(VncState *vs); +-- +2.11.0 diff --git a/meta/recipes-devtools/qemu/qemu_2.11.0.bb b/meta/recipes-devtools/qemu/qemu_2.11.0.bb index 8306db2ce5..10760dc2db 100644 --- a/meta/recipes-devtools/qemu/qemu_2.11.0.bb +++ b/meta/recipes-devtools/qemu/qemu_2.11.0.bb @@ -22,6 +22,7 @@ SRC_URI = "http://wiki.qemu-project.org/download/${BP}.tar.bz2 \ file://apic-fixup-fallthrough-to-PIC.patch \ file://linux-user-Fix-webkitgtk-hangs-on-32-bit-x86-target.patch \ file://memfd.patch \ + file://CVE-2017-15124.patch \ " UPSTREAM_CHECK_REGEX = "qemu-(?P<pver>\d+\..*)\.tar" |