* [Qemu-devel] [PATCH] [sparse] fix build
2014-10-15 10:10 [Qemu-devel] [PATCH v2 0/5] vmware-vga: fix CVE-2014-3689 Gerd Hoffmann
@ 2014-10-15 10:10 ` Gerd Hoffmann
2014-10-15 10:13 ` Gerd Hoffmann
2014-10-15 10:10 ` [Qemu-devel] [PATCH v2 1/5] vmware-vga: CVE-2014-3689: turn off hw accel Gerd Hoffmann
` (5 subsequent siblings)
6 siblings, 1 reply; 16+ messages in thread
From: Gerd Hoffmann @ 2014-10-15 10:10 UTC (permalink / raw)
To: qemu-devel
Cc: pmatouse, Intel.Product.Security.Incident.Response.Team,
Gerd Hoffmann
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
configure | 1 +
1 file changed, 1 insertion(+)
diff --git a/configure b/configure
index 9802e89..060c42a 100755
--- a/configure
+++ b/configure
@@ -4939,6 +4939,7 @@ echo "QEMU_CFLAGS=$QEMU_CFLAGS" >> $config_host_mak
echo "QEMU_INCLUDES=$QEMU_INCLUDES" >> $config_host_mak
if test "$sparse" = "yes" ; then
echo "CC := REAL_CC=\"\$(CC)\" cgcc" >> $config_host_mak
+ echo "CXX := REAL_CC=\"\$(CXX)\" cgcc" >> $config_host_mak
echo "HOST_CC := REAL_CC=\"\$(HOST_CC)\" cgcc" >> $config_host_mak
echo "QEMU_CFLAGS += -Wbitwise -Wno-transparent-union -Wno-old-initializer -Wno-non-pointer-null" >> $config_host_mak
fi
--
1.8.3.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH v2 1/5] vmware-vga: CVE-2014-3689: turn off hw accel
2014-10-15 10:10 [Qemu-devel] [PATCH v2 0/5] vmware-vga: fix CVE-2014-3689 Gerd Hoffmann
2014-10-15 10:10 ` [Qemu-devel] [PATCH] [sparse] fix build Gerd Hoffmann
@ 2014-10-15 10:10 ` Gerd Hoffmann
2014-10-16 14:25 ` Don Koch
2014-10-15 10:10 ` [Qemu-devel] [PATCH v2 2/5] vmware-vga: add vmsvga_verify_rect Gerd Hoffmann
` (4 subsequent siblings)
6 siblings, 1 reply; 16+ messages in thread
From: Gerd Hoffmann @ 2014-10-15 10:10 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-stable, pmatouse,
Intel.Product.Security.Incident.Response.Team, Gerd Hoffmann
Quick & easy stopgap for CVE-2014-3689: We just compile out the
hardware acceleration functions which lack sanity checks. Thankfully
we have capability bits for them (SVGA_CAP_RECT_COPY and
SVGA_CAP_RECT_FILL), so guests should deal just fine, in theory.
Subsequent patches will add the missing checks and re-enable the
hardware acceleration emulation.
Cc: qemu-stable@nongnu.org
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
hw/display/vmware_vga.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/hw/display/vmware_vga.c b/hw/display/vmware_vga.c
index 0c36c72..ec63290 100644
--- a/hw/display/vmware_vga.c
+++ b/hw/display/vmware_vga.c
@@ -29,8 +29,10 @@
#include "hw/pci/pci.h"
#undef VERBOSE
+#if 0
#define HW_RECT_ACCEL
#define HW_FILL_ACCEL
+#endif
#define HW_MOUSE_ACCEL
#include "vga_int.h"
--
1.8.3.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH v2 1/5] vmware-vga: CVE-2014-3689: turn off hw accel
2014-10-15 10:10 ` [Qemu-devel] [PATCH v2 1/5] vmware-vga: CVE-2014-3689: turn off hw accel Gerd Hoffmann
@ 2014-10-16 14:25 ` Don Koch
0 siblings, 0 replies; 16+ messages in thread
From: Don Koch @ 2014-10-16 14:25 UTC (permalink / raw)
To: Gerd Hoffmann
Cc: Intel.Product.Security.Incident.Response.Team, pmatouse,
qemu-devel, qemu-stable
On Wed, 15 Oct 2014 12:10:35 +0200
Gerd Hoffmann <kraxel@redhat.com> wrote:
> Quick & easy stopgap for CVE-2014-3689: We just compile out the
> hardware acceleration functions which lack sanity checks. Thankfully
> we have capability bits for them (SVGA_CAP_RECT_COPY and
> SVGA_CAP_RECT_FILL), so guests should deal just fine, in theory.
>
> Subsequent patches will add the missing checks and re-enable the
> hardware acceleration emulation.
>
> Cc: qemu-stable@nongnu.org
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Don Koch <dkoch@verizon.com>
> ---
> hw/display/vmware_vga.c | 2 ++
> 1 file changed, 2 insertions(+)
>
> diff --git a/hw/display/vmware_vga.c b/hw/display/vmware_vga.c
> index 0c36c72..ec63290 100644
> --- a/hw/display/vmware_vga.c
> +++ b/hw/display/vmware_vga.c
> @@ -29,8 +29,10 @@
> #include "hw/pci/pci.h"
>
> #undef VERBOSE
> +#if 0
> #define HW_RECT_ACCEL
> #define HW_FILL_ACCEL
> +#endif
> #define HW_MOUSE_ACCEL
>
> #include "vga_int.h"
> --
> 1.8.3.1
>
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH v2 2/5] vmware-vga: add vmsvga_verify_rect
2014-10-15 10:10 [Qemu-devel] [PATCH v2 0/5] vmware-vga: fix CVE-2014-3689 Gerd Hoffmann
2014-10-15 10:10 ` [Qemu-devel] [PATCH] [sparse] fix build Gerd Hoffmann
2014-10-15 10:10 ` [Qemu-devel] [PATCH v2 1/5] vmware-vga: CVE-2014-3689: turn off hw accel Gerd Hoffmann
@ 2014-10-15 10:10 ` Gerd Hoffmann
2014-10-16 15:19 ` Don Koch
2014-10-15 10:10 ` [Qemu-devel] [PATCH v2 3/5] vmware-vga: use vmsvga_verify_rect in vmsvga_update_rect Gerd Hoffmann
` (3 subsequent siblings)
6 siblings, 1 reply; 16+ messages in thread
From: Gerd Hoffmann @ 2014-10-15 10:10 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-stable, pmatouse,
Intel.Product.Security.Incident.Response.Team, Gerd Hoffmann
Add verification function for rectangles, returning
true if verification passes and false otherwise.
Cc: qemu-stable@nongnu.org
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
hw/display/vmware_vga.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++++-
1 file changed, 52 insertions(+), 1 deletion(-)
diff --git a/hw/display/vmware_vga.c b/hw/display/vmware_vga.c
index ec63290..ba73a1c 100644
--- a/hw/display/vmware_vga.c
+++ b/hw/display/vmware_vga.c
@@ -294,8 +294,59 @@ enum {
SVGA_CURSOR_ON_RESTORE_TO_FB = 3,
};
+static inline bool vmsvga_verify_rect(DisplaySurface *surface,
+ const char *name,
+ int x, int y, int w, int h)
+{
+ if (x < 0) {
+ fprintf(stderr, "%s: x was < 0 (%d)\n", name, x);
+ return false;
+ }
+ if (x > SVGA_MAX_WIDTH) {
+ fprintf(stderr, "%s: x was > %d (%d)\n", name, SVGA_MAX_WIDTH, x);
+ return false;
+ }
+ if (w < 0) {
+ fprintf(stderr, "%s: w was < 0 (%d)\n", name, w);
+ return false;
+ }
+ if (w > SVGA_MAX_WIDTH) {
+ fprintf(stderr, "%s: w was > %d (%d)\n", name, SVGA_MAX_WIDTH, w);
+ return false;
+ }
+ if (x + w > surface_width(surface)) {
+ fprintf(stderr, "%s: width was > %d (x: %d, w: %d)\n",
+ name, surface_width(surface), x, w);
+ return false;
+ }
+
+ if (y < 0) {
+ fprintf(stderr, "%s: y was < 0 (%d)\n", name, y);
+ return false;
+ }
+ if (y > SVGA_MAX_HEIGHT) {
+ fprintf(stderr, "%s: y was > %d (%d)\n", name, SVGA_MAX_HEIGHT, y);
+ return false;
+ }
+ if (h < 0) {
+ fprintf(stderr, "%s: h was < 0 (%d)\n", name, h);
+ return false;
+ }
+ if (h > SVGA_MAX_HEIGHT) {
+ fprintf(stderr, "%s: h was > %d (%d)\n", name, SVGA_MAX_HEIGHT, h);
+ return false;
+ }
+ if (y + h > surface_height(surface)) {
+ fprintf(stderr, "%s: update height > %d (y: %d, h: %d)\n",
+ name, surface_height(surface), y, h);
+ return false;
+ }
+
+ return true;
+}
+
static inline void vmsvga_update_rect(struct vmsvga_state_s *s,
- int x, int y, int w, int h)
+ int x, int y, int w, int h)
{
DisplaySurface *surface = qemu_console_surface(s->vga.con);
int line;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH v2 2/5] vmware-vga: add vmsvga_verify_rect
2014-10-15 10:10 ` [Qemu-devel] [PATCH v2 2/5] vmware-vga: add vmsvga_verify_rect Gerd Hoffmann
@ 2014-10-16 15:19 ` Don Koch
0 siblings, 0 replies; 16+ messages in thread
From: Don Koch @ 2014-10-16 15:19 UTC (permalink / raw)
To: Gerd Hoffmann
Cc: Intel.Product.Security.Incident.Response.Team, pmatouse,
qemu-devel, qemu-stable
On Wed, 15 Oct 2014 12:10:36 +0200
Gerd Hoffmann <kraxel@redhat.com> wrote:
> Add verification function for rectangles, returning
> true if verification passes and false otherwise.
>
> Cc: qemu-stable@nongnu.org
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Don Koch <dkoch@verizon.com>
> ---
> hw/display/vmware_vga.c | 53 ++++++++++++++++++++++++++++++++++++++++++++++++-
> 1 file changed, 52 insertions(+), 1 deletion(-)
>
> diff --git a/hw/display/vmware_vga.c b/hw/display/vmware_vga.c
> index ec63290..ba73a1c 100644
> --- a/hw/display/vmware_vga.c
> +++ b/hw/display/vmware_vga.c
> @@ -294,8 +294,59 @@ enum {
> SVGA_CURSOR_ON_RESTORE_TO_FB = 3,
> };
>
> +static inline bool vmsvga_verify_rect(DisplaySurface *surface,
> + const char *name,
> + int x, int y, int w, int h)
> +{
> + if (x < 0) {
> + fprintf(stderr, "%s: x was < 0 (%d)\n", name, x);
> + return false;
> + }
> + if (x > SVGA_MAX_WIDTH) {
> + fprintf(stderr, "%s: x was > %d (%d)\n", name, SVGA_MAX_WIDTH, x);
> + return false;
> + }
> + if (w < 0) {
> + fprintf(stderr, "%s: w was < 0 (%d)\n", name, w);
> + return false;
> + }
> + if (w > SVGA_MAX_WIDTH) {
> + fprintf(stderr, "%s: w was > %d (%d)\n", name, SVGA_MAX_WIDTH, w);
> + return false;
> + }
> + if (x + w > surface_width(surface)) {
> + fprintf(stderr, "%s: width was > %d (x: %d, w: %d)\n",
> + name, surface_width(surface), x, w);
> + return false;
> + }
> +
> + if (y < 0) {
> + fprintf(stderr, "%s: y was < 0 (%d)\n", name, y);
> + return false;
> + }
> + if (y > SVGA_MAX_HEIGHT) {
> + fprintf(stderr, "%s: y was > %d (%d)\n", name, SVGA_MAX_HEIGHT, y);
> + return false;
> + }
> + if (h < 0) {
> + fprintf(stderr, "%s: h was < 0 (%d)\n", name, h);
> + return false;
> + }
> + if (h > SVGA_MAX_HEIGHT) {
> + fprintf(stderr, "%s: h was > %d (%d)\n", name, SVGA_MAX_HEIGHT, h);
> + return false;
> + }
> + if (y + h > surface_height(surface)) {
> + fprintf(stderr, "%s: update height > %d (y: %d, h: %d)\n",
> + name, surface_height(surface), y, h);
> + return false;
> + }
> +
> + return true;
> +}
> +
> static inline void vmsvga_update_rect(struct vmsvga_state_s *s,
> - int x, int y, int w, int h)
> + int x, int y, int w, int h)
> {
> DisplaySurface *surface = qemu_console_surface(s->vga.con);
> int line;
> --
> 1.8.3.1
>
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH v2 3/5] vmware-vga: use vmsvga_verify_rect in vmsvga_update_rect
2014-10-15 10:10 [Qemu-devel] [PATCH v2 0/5] vmware-vga: fix CVE-2014-3689 Gerd Hoffmann
` (2 preceding siblings ...)
2014-10-15 10:10 ` [Qemu-devel] [PATCH v2 2/5] vmware-vga: add vmsvga_verify_rect Gerd Hoffmann
@ 2014-10-15 10:10 ` Gerd Hoffmann
2014-10-16 14:25 ` Don Koch
2014-10-15 10:10 ` [Qemu-devel] [PATCH v2 4/5] vmware-vga: use vmsvga_verify_rect in vmsvga_copy_rect Gerd Hoffmann
` (2 subsequent siblings)
6 siblings, 1 reply; 16+ messages in thread
From: Gerd Hoffmann @ 2014-10-15 10:10 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-stable, pmatouse,
Intel.Product.Security.Incident.Response.Team, Gerd Hoffmann
Switch vmsvga_update_rect over to use vmsvga_verify_rect. Slight change
in behavior: We don't try to automatically fixup rectangles any more.
In case we find invalid update requests we'll do a full-screen update
instead.
Cc: qemu-stable@nongnu.org
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
hw/display/vmware_vga.c | 32 ++++----------------------------
1 file changed, 4 insertions(+), 28 deletions(-)
diff --git a/hw/display/vmware_vga.c b/hw/display/vmware_vga.c
index ba73a1c..9d79de6 100644
--- a/hw/display/vmware_vga.c
+++ b/hw/display/vmware_vga.c
@@ -356,36 +356,12 @@ static inline void vmsvga_update_rect(struct vmsvga_state_s *s,
uint8_t *src;
uint8_t *dst;
- if (x < 0) {
- fprintf(stderr, "%s: update x was < 0 (%d)\n", __func__, x);
- w += x;
+ if (!vmsvga_verify_rect(surface, __func__, x, y, w, h)) {
+ /* go for a fullscreen update as fallback */
x = 0;
- }
- if (w < 0) {
- fprintf(stderr, "%s: update w was < 0 (%d)\n", __func__, w);
- w = 0;
- }
- if (x + w > surface_width(surface)) {
- fprintf(stderr, "%s: update width too large x: %d, w: %d\n",
- __func__, x, w);
- x = MIN(x, surface_width(surface));
- w = surface_width(surface) - x;
- }
-
- if (y < 0) {
- fprintf(stderr, "%s: update y was < 0 (%d)\n", __func__, y);
- h += y;
y = 0;
- }
- if (h < 0) {
- fprintf(stderr, "%s: update h was < 0 (%d)\n", __func__, h);
- h = 0;
- }
- if (y + h > surface_height(surface)) {
- fprintf(stderr, "%s: update height too large y: %d, h: %d\n",
- __func__, y, h);
- y = MIN(y, surface_height(surface));
- h = surface_height(surface) - y;
+ w = surface_width(surface);
+ h = surface_height(surface);
}
bypl = surface_stride(surface);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH v2 3/5] vmware-vga: use vmsvga_verify_rect in vmsvga_update_rect
2014-10-15 10:10 ` [Qemu-devel] [PATCH v2 3/5] vmware-vga: use vmsvga_verify_rect in vmsvga_update_rect Gerd Hoffmann
@ 2014-10-16 14:25 ` Don Koch
0 siblings, 0 replies; 16+ messages in thread
From: Don Koch @ 2014-10-16 14:25 UTC (permalink / raw)
To: Gerd Hoffmann
Cc: Intel.Product.Security.Incident.Response.Team, pmatouse,
qemu-devel, qemu-stable
On Wed, 15 Oct 2014 12:10:37 +0200
Gerd Hoffmann <kraxel@redhat.com> wrote:
> Switch vmsvga_update_rect over to use vmsvga_verify_rect. Slight change
> in behavior: We don't try to automatically fixup rectangles any more.
> In case we find invalid update requests we'll do a full-screen update
> instead.
This is good since the original calculations were wrong. (I had already fixed
said calculations but hadn't cleaned them up for submittal, yet.) Unfortunate
that you end up using "the big hammer" to fix it (i.e., update the entire screen),
but that's better than before.
Reviewed-by: Don Koch <dkoch@verizon.com>
> Cc: qemu-stable@nongnu.org
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
> hw/display/vmware_vga.c | 32 ++++----------------------------
> 1 file changed, 4 insertions(+), 28 deletions(-)
>
> diff --git a/hw/display/vmware_vga.c b/hw/display/vmware_vga.c
> index ba73a1c..9d79de6 100644
> --- a/hw/display/vmware_vga.c
> +++ b/hw/display/vmware_vga.c
> @@ -356,36 +356,12 @@ static inline void vmsvga_update_rect(struct vmsvga_state_s *s,
> uint8_t *src;
> uint8_t *dst;
>
> - if (x < 0) {
> - fprintf(stderr, "%s: update x was < 0 (%d)\n", __func__, x);
> - w += x;
> + if (!vmsvga_verify_rect(surface, __func__, x, y, w, h)) {
> + /* go for a fullscreen update as fallback */
> x = 0;
> - }
> - if (w < 0) {
> - fprintf(stderr, "%s: update w was < 0 (%d)\n", __func__, w);
> - w = 0;
> - }
> - if (x + w > surface_width(surface)) {
> - fprintf(stderr, "%s: update width too large x: %d, w: %d\n",
> - __func__, x, w);
> - x = MIN(x, surface_width(surface));
> - w = surface_width(surface) - x;
> - }
> -
> - if (y < 0) {
> - fprintf(stderr, "%s: update y was < 0 (%d)\n", __func__, y);
> - h += y;
> y = 0;
> - }
> - if (h < 0) {
> - fprintf(stderr, "%s: update h was < 0 (%d)\n", __func__, h);
> - h = 0;
> - }
> - if (y + h > surface_height(surface)) {
> - fprintf(stderr, "%s: update height too large y: %d, h: %d\n",
> - __func__, y, h);
> - y = MIN(y, surface_height(surface));
> - h = surface_height(surface) - y;
> + w = surface_width(surface);
> + h = surface_height(surface);
> }
>
> bypl = surface_stride(surface);
> --
> 1.8.3.1
>
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH v2 4/5] vmware-vga: use vmsvga_verify_rect in vmsvga_copy_rect
2014-10-15 10:10 [Qemu-devel] [PATCH v2 0/5] vmware-vga: fix CVE-2014-3689 Gerd Hoffmann
` (3 preceding siblings ...)
2014-10-15 10:10 ` [Qemu-devel] [PATCH v2 3/5] vmware-vga: use vmsvga_verify_rect in vmsvga_update_rect Gerd Hoffmann
@ 2014-10-15 10:10 ` Gerd Hoffmann
2014-10-16 14:29 ` Don Koch
2014-10-15 10:10 ` [Qemu-devel] [PATCH v2 5/5] vmware-vga: use vmsvga_verify_rect in vmsvga_fill_rect Gerd Hoffmann
2014-10-15 15:43 ` [Qemu-devel] [PATCH v2 0/5] vmware-vga: fix CVE-2014-3689 Michael Tokarev
6 siblings, 1 reply; 16+ messages in thread
From: Gerd Hoffmann @ 2014-10-15 10:10 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-stable, pmatouse,
Intel.Product.Security.Incident.Response.Team, Gerd Hoffmann
Add verification to vmsvga_copy_rect, re-enable HW_RECT_ACCEL.
Cc: qemu-stable@nongnu.org
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
hw/display/vmware_vga.c | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)
diff --git a/hw/display/vmware_vga.c b/hw/display/vmware_vga.c
index 9d79de6..1fc9641 100644
--- a/hw/display/vmware_vga.c
+++ b/hw/display/vmware_vga.c
@@ -29,8 +29,8 @@
#include "hw/pci/pci.h"
#undef VERBOSE
-#if 0
#define HW_RECT_ACCEL
+#if 0
#define HW_FILL_ACCEL
#endif
#define HW_MOUSE_ACCEL
@@ -417,6 +417,13 @@ static inline void vmsvga_copy_rect(struct vmsvga_state_s *s,
int line = h;
uint8_t *ptr[2];
+ if (!vmsvga_verify_rect(surface, "vmsvga_copy_rect/src", x0, y0, w, h)) {
+ return;
+ }
+ if (!vmsvga_verify_rect(surface, "vmsvga_copy_rect/dst", x1, y1, w, h)) {
+ return;
+ }
+
if (y1 > y0) {
ptr[0] = vram + bypp * x0 + bypl * (y0 + h - 1);
ptr[1] = vram + bypp * x1 + bypl * (y1 + h - 1);
--
1.8.3.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH v2 4/5] vmware-vga: use vmsvga_verify_rect in vmsvga_copy_rect
2014-10-15 10:10 ` [Qemu-devel] [PATCH v2 4/5] vmware-vga: use vmsvga_verify_rect in vmsvga_copy_rect Gerd Hoffmann
@ 2014-10-16 14:29 ` Don Koch
0 siblings, 0 replies; 16+ messages in thread
From: Don Koch @ 2014-10-16 14:29 UTC (permalink / raw)
To: Gerd Hoffmann
Cc: Intel.Product.Security.Incident.Response.Team, pmatouse,
qemu-devel, qemu-stable
On Wed, 15 Oct 2014 12:10:38 +0200
Gerd Hoffmann <kraxel@redhat.com> wrote:
> Add verification to vmsvga_copy_rect, re-enable HW_RECT_ACCEL.
>
> Cc: qemu-stable@nongnu.org
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
> hw/display/vmware_vga.c | 9 ++++++++-
> 1 file changed, 8 insertions(+), 1 deletion(-)
>
> diff --git a/hw/display/vmware_vga.c b/hw/display/vmware_vga.c
> index 9d79de6..1fc9641 100644
> --- a/hw/display/vmware_vga.c
> +++ b/hw/display/vmware_vga.c
> @@ -29,8 +29,8 @@
> #include "hw/pci/pci.h"
>
> #undef VERBOSE
> -#if 0
> #define HW_RECT_ACCEL
> +#if 0
> #define HW_FILL_ACCEL
> #endif
> #define HW_MOUSE_ACCEL
> @@ -417,6 +417,13 @@ static inline void vmsvga_copy_rect(struct vmsvga_state_s *s,
> int line = h;
> uint8_t *ptr[2];
>
> + if (!vmsvga_verify_rect(surface, "vmsvga_copy_rect/src", x0, y0, w, h)) {
> + return;
> + }
> + if (!vmsvga_verify_rect(surface, "vmsvga_copy_rect/dst", x1, y1, w, h)) {
> + return;
> + }
> +
If I read this correctly, if either the source or destination are even partially
off-screen, the copy silently fails, which sounds wrong.
I'd suggest having this function return false if one of these checks fail so the
caller can do something appropriate (like "goto badcmd").
-d
> if (y1 > y0) {
> ptr[0] = vram + bypp * x0 + bypl * (y0 + h - 1);
> ptr[1] = vram + bypp * x1 + bypl * (y1 + h - 1);
> --
> 1.8.3.1
>
>
^ permalink raw reply [flat|nested] 16+ messages in thread
* [Qemu-devel] [PATCH v2 5/5] vmware-vga: use vmsvga_verify_rect in vmsvga_fill_rect
2014-10-15 10:10 [Qemu-devel] [PATCH v2 0/5] vmware-vga: fix CVE-2014-3689 Gerd Hoffmann
` (4 preceding siblings ...)
2014-10-15 10:10 ` [Qemu-devel] [PATCH v2 4/5] vmware-vga: use vmsvga_verify_rect in vmsvga_copy_rect Gerd Hoffmann
@ 2014-10-15 10:10 ` Gerd Hoffmann
2014-10-16 15:21 ` Don Koch
2014-10-15 15:43 ` [Qemu-devel] [PATCH v2 0/5] vmware-vga: fix CVE-2014-3689 Michael Tokarev
6 siblings, 1 reply; 16+ messages in thread
From: Gerd Hoffmann @ 2014-10-15 10:10 UTC (permalink / raw)
To: qemu-devel
Cc: qemu-stable, pmatouse,
Intel.Product.Security.Incident.Response.Team, Gerd Hoffmann
Add verification to vmsvga_fill_rect, re-enable HW_FILL_ACCEL.
Cc: qemu-stable@nongnu.org
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
hw/display/vmware_vga.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/hw/display/vmware_vga.c b/hw/display/vmware_vga.c
index 1fc9641..7f3a9e6 100644
--- a/hw/display/vmware_vga.c
+++ b/hw/display/vmware_vga.c
@@ -30,9 +30,7 @@
#undef VERBOSE
#define HW_RECT_ACCEL
-#if 0
#define HW_FILL_ACCEL
-#endif
#define HW_MOUSE_ACCEL
#include "vga_int.h"
@@ -456,6 +454,10 @@ static inline void vmsvga_fill_rect(struct vmsvga_state_s *s,
uint8_t *src;
uint8_t col[4];
+ if (!vmsvga_verify_rect(surface, __func__, x, y, w, h)) {
+ return;
+ }
+
col[0] = c;
col[1] = c >> 8;
col[2] = c >> 16;
--
1.8.3.1
^ permalink raw reply related [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH v2 5/5] vmware-vga: use vmsvga_verify_rect in vmsvga_fill_rect
2014-10-15 10:10 ` [Qemu-devel] [PATCH v2 5/5] vmware-vga: use vmsvga_verify_rect in vmsvga_fill_rect Gerd Hoffmann
@ 2014-10-16 15:21 ` Don Koch
0 siblings, 0 replies; 16+ messages in thread
From: Don Koch @ 2014-10-16 15:21 UTC (permalink / raw)
To: Gerd Hoffmann
Cc: Intel.Product.Security.Incident.Response.Team, pmatouse,
qemu-devel, qemu-stable
On Wed, 15 Oct 2014 12:10:39 +0200
Gerd Hoffmann <kraxel@redhat.com> wrote:
> Add verification to vmsvga_fill_rect, re-enable HW_FILL_ACCEL.
>
> Cc: qemu-stable@nongnu.org
> Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
> ---
> hw/display/vmware_vga.c | 6 ++++--
> 1 file changed, 4 insertions(+), 2 deletions(-)
>
> diff --git a/hw/display/vmware_vga.c b/hw/display/vmware_vga.c
> index 1fc9641..7f3a9e6 100644
> --- a/hw/display/vmware_vga.c
> +++ b/hw/display/vmware_vga.c
> @@ -30,9 +30,7 @@
>
> #undef VERBOSE
> #define HW_RECT_ACCEL
> -#if 0
> #define HW_FILL_ACCEL
> -#endif
> #define HW_MOUSE_ACCEL
>
> #include "vga_int.h"
> @@ -456,6 +454,10 @@ static inline void vmsvga_fill_rect(struct vmsvga_state_s *s,
> uint8_t *src;
> uint8_t col[4];
>
> + if (!vmsvga_verify_rect(surface, __func__, x, y, w, h)) {
> + return;
> + }
> +
Same issue as with vmsvga_copy_rect().
-d
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH v2 0/5] vmware-vga: fix CVE-2014-3689
2014-10-15 10:10 [Qemu-devel] [PATCH v2 0/5] vmware-vga: fix CVE-2014-3689 Gerd Hoffmann
` (5 preceding siblings ...)
2014-10-15 10:10 ` [Qemu-devel] [PATCH v2 5/5] vmware-vga: use vmsvga_verify_rect in vmsvga_fill_rect Gerd Hoffmann
@ 2014-10-15 15:43 ` Michael Tokarev
2014-10-16 10:54 ` Gerd Hoffmann
6 siblings, 1 reply; 16+ messages in thread
From: Michael Tokarev @ 2014-10-15 15:43 UTC (permalink / raw)
To: Gerd Hoffmann, qemu-devel
Cc: pmatouse, Intel.Product.Security.Incident.Response.Team
On 15.10.2014 12:10, Gerd Hoffmann wrote:
> Hi,
>
> vmware-vga emulation lacks sanity checks in the hardware acceleration
> (blit + fill) functions. This patch series plugs the holes.
>
> v2 changes:
> * small whitespace fixup.
> * do fullscreen update on invalid update requests.
>
> cheers,
> Gerd
>
> Gerd Hoffmann (5):
> vmware-vga: CVE-2014-3689: turn off hw accel
> vmware-vga: add vmsvga_verify_rect
> vmware-vga: use vmsvga_verify_rect in vmsvga_update_rect
> vmware-vga: use vmsvga_verify_rect in vmsvga_copy_rect
> vmware-vga: use vmsvga_verify_rect in vmsvga_fill_rect
A small question. Why do you first disable the hw accel for rect&fill
and re-enable them in subsequent patches, as if applying the real
fix patches takes very long time and during that time we need the
hole to be fixed? Why not just to fix it for real without the temp
workarounds? :)
Thanks,
/mjt
^ permalink raw reply [flat|nested] 16+ messages in thread
* Re: [Qemu-devel] [PATCH v2 0/5] vmware-vga: fix CVE-2014-3689
2014-10-15 15:43 ` [Qemu-devel] [PATCH v2 0/5] vmware-vga: fix CVE-2014-3689 Michael Tokarev
@ 2014-10-16 10:54 ` Gerd Hoffmann
0 siblings, 0 replies; 16+ messages in thread
From: Gerd Hoffmann @ 2014-10-16 10:54 UTC (permalink / raw)
To: Michael Tokarev
Cc: pmatouse, qemu-devel,
Intel.Product.Security.Incident.Response.Team
On Mi, 2014-10-15 at 17:43 +0200, Michael Tokarev wrote:
> On 15.10.2014 12:10, Gerd Hoffmann wrote:
> > Hi,
> >
> > vmware-vga emulation lacks sanity checks in the hardware acceleration
> > (blit + fill) functions. This patch series plugs the holes.
> >
> > v2 changes:
> > * small whitespace fixup.
> > * do fullscreen update on invalid update requests.
> >
> > cheers,
> > Gerd
> >
> > Gerd Hoffmann (5):
> > vmware-vga: CVE-2014-3689: turn off hw accel
> > vmware-vga: add vmsvga_verify_rect
> > vmware-vga: use vmsvga_verify_rect in vmsvga_update_rect
> > vmware-vga: use vmsvga_verify_rect in vmsvga_copy_rect
> > vmware-vga: use vmsvga_verify_rect in vmsvga_fill_rect
>
> A small question. Why do you first disable the hw accel for rect&fill
> and re-enable them in subsequent patches, as if applying the real
> fix patches takes very long time and during that time we need the
> hole to be fixed?
That was just the order the patches where created. There isn't a real
need for patch #1, but it didn't look important enough to me to bother
fixing it up after the series was complete.
cheers,
Gerd
^ permalink raw reply [flat|nested] 16+ messages in thread