cryptospore / rpms / qemu-kvm

Forked from rpms/qemu-kvm 2 years ago
Clone

Blame SOURCES/kvm-ui-avoid-sign-extension-using-client-width-height.patch

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