qemu: fix CVE-2017-15124

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.

Backport a series of patches from upstream to resolve this.

Signed-off-by: Ross Burton <ross.burton@intel.com>

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: Ma