yeahuh / rpms / qemu-kvm

Forked from rpms/qemu-kvm 2 years ago
Clone
5d360b
From 023a741e196fd99b532f69df9560730425b174a0 Mon Sep 17 00:00:00 2001
5d360b
From: "Daniel P. Berrange" <berrange@redhat.com>
5d360b
Date: Thu, 8 Feb 2018 17:50:39 +0100
5d360b
Subject: [PATCH 25/27] ui: avoid sign extension using client width/height
5d360b
5d360b
RH-Author: Daniel P. Berrange <berrange@redhat.com>
5d360b
Message-id: <20180208175041.5634-26-berrange@redhat.com>
5d360b
Patchwork-id: 78960
5d360b
O-Subject: [RHEL-7.5 qemu-kvm PATCH v1 25/27] ui: avoid sign extension using client width/height
5d360b
Bugzilla: 1527405
5d360b
RH-Acked-by: Laszlo Ersek <lersek@redhat.com>
5d360b
RH-Acked-by: Gerd Hoffmann <kraxel@redhat.com>
5d360b
RH-Acked-by: Miroslav Rezanina <mrezanin@redhat.com>
5d360b
5d360b
From: "Daniel P. Berrange" <berrange@redhat.com>
5d360b
5d360b
Pixman returns a signed int for the image width/height, but the VNC
5d360b
protocol only permits a unsigned int16. Effective framebuffer size
5d360b
is determined by the guest, limited by the video RAM size, so the
5d360b
dimensions are unlikely to exceed the range of an unsigned int16,
5d360b
but this is not currently validated.
5d360b
5d360b
With the current use of 'int' for client width/height, the calculation
5d360b
of offsets in vnc_update_throttle_offset() suffers from integer size
5d360b
promotion and sign extension, causing coverity warnings
5d360b
5d360b
*** CID 1385147:  Integer handling issues  (SIGN_EXTENSION)
5d360b
/ui/vnc.c: 979 in vnc_update_throttle_offset()
5d360b
973      * than that the client would already suffering awful audio
5d360b
974      * glitches, so dropping samples is no worse really).
5d360b
975      */
5d360b
976     static void vnc_update_throttle_offset(VncState *vs)
5d360b
977     {
5d360b
978         size_t offset =
5d360b
>>>     CID 1385147:  Integer handling issues  (SIGN_EXTENSION)
5d360b
>>>     Suspicious implicit sign extension:
5d360b
    "vs->client_pf.bytes_per_pixel" with type "unsigned char" (8 bits,
5d360b
    unsigned) is promoted in "vs->client_width * vs->client_height *
5d360b
    vs->client_pf.bytes_per_pixel" to type "int" (32 bits, signed), then
5d360b
    sign-extended to type "unsigned long" (64 bits, unsigned).  If
5d360b
    "vs->client_width * vs->client_height * vs->client_pf.bytes_per_pixel"
5d360b
    is greater than 0x7FFFFFFF, the upper bits of the result will all be 1.
5d360b
979             vs->client_width * vs->client_height * vs->client_pf.bytes_per_pixel;
5d360b
5d360b
Change client_width / client_height to be a size_t to avoid sign
5d360b
extension and integer promotion. Then validate that dimensions are in
5d360b
range wrt the RFB protocol u16 limits.
5d360b
5d360b
Signed-off-by: Daniel P. Berrange <berrange@redhat.com>
5d360b
Message-id: 20180118155254.17053-1-berrange@redhat.com
5d360b
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
5d360b
(cherry picked from commit 4c956bd81e2e16afd19d38d1fdeba6d9faa8a1ae)
5d360b
5d360b
 Conflicts:
5d360b
	ui/vnc.c
5d360b
	ui/vnc.h - context differences
5d360b
5d360b
RHEL-7 note: The context difference in VncState is because downstream
5d360b
lacks commit 14768eba46e4 ("input: mouse: switch vnc ui to new core",
5d360b
2014-03-05), part of v2.0.0. The context difference in
5d360b
protocol_client_init() is because downstream lacks e5f34cdd2da5 ("vnc:
5d360b
track & limit connections", 2015-01-22), part of v2.3.0. Neither
5d360b
upstream commit is necessary for this backport.
5d360b
5d360b
Signed-off-by: Miroslav Rezanina <mrezanin@redhat.com>
5d360b
---
5d360b
 ui/vnc.c | 9 +++++++++
5d360b
 ui/vnc.h | 4 ++--
5d360b
 2 files changed, 11 insertions(+), 2 deletions(-)
5d360b
5d360b
diff --git a/ui/vnc.c b/ui/vnc.c
5d360b
index 61fbec2..2be87b8 100644
5d360b
--- a/ui/vnc.c
5d360b
+++ b/ui/vnc.c
5d360b
@@ -570,6 +570,11 @@ static void vnc_desktop_resize(VncState *vs)
5d360b
         vs->client_height == pixman_image_get_height(vs->vd->server)) {
5d360b
         return;
5d360b
     }
5d360b
+
5d360b
+    assert(pixman_image_get_width(vs->vd->server) < 65536 &&
5d360b
+           pixman_image_get_width(vs->vd->server) >= 0);
5d360b
+    assert(pixman_image_get_height(vs->vd->server) < 65536 &&
5d360b
+           pixman_image_get_height(vs->vd->server) >= 0);
5d360b
     vs->client_width = pixman_image_get_width(vs->vd->server);
5d360b
     vs->client_height = pixman_image_get_height(vs->vd->server);
5d360b
     vnc_lock_output(vs);
5d360b
@@ -2387,6 +2392,10 @@ static int protocol_client_init(VncState *vs, uint8_t *data, size_t len)
5d360b
     }
5d360b
     vnc_set_share_mode(vs, mode);
5d360b
 
5d360b
+    assert(pixman_image_get_width(vs->vd->server) < 65536 &&
5d360b
+           pixman_image_get_width(vs->vd->server) >= 0);
5d360b
+    assert(pixman_image_get_height(vs->vd->server) < 65536 &&
5d360b
+           pixman_image_get_height(vs->vd->server) >= 0);
5d360b
     vs->client_width = pixman_image_get_width(vs->vd->server);
5d360b
     vs->client_height = pixman_image_get_height(vs->vd->server);
5d360b
     vnc_write_u16(vs, vs->client_width);
5d360b
diff --git a/ui/vnc.h b/ui/vnc.h
5d360b
index 70316ba..f9c5f89 100644
5d360b
--- a/ui/vnc.h
5d360b
+++ b/ui/vnc.h
5d360b
@@ -274,8 +274,8 @@ struct VncState
5d360b
     int absolute;
5d360b
     int last_x;
5d360b
     int last_y;
5d360b
-    int client_width;
5d360b
-    int client_height;
5d360b
+    size_t client_width; /* limited to u16 by RFB proto */
5d360b
+    size_t client_height; /* limited to u16 by RFB proto */
5d360b
     VncShareMode share_mode;
5d360b
 
5d360b
     uint32_t vnc_encoding;
5d360b
-- 
5d360b
1.8.3.1
5d360b