[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
[Qemu-devel] Re: [PATCH to consider for 0.12] vmware_vga: Don't crash on
From: |
Roland Dreier |
Subject: |
[Qemu-devel] Re: [PATCH to consider for 0.12] vmware_vga: Don't crash on too-big DEFINE_CURSOR command |
Date: |
Thu, 17 Dec 2009 14:38:08 -0800 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/23.1 (gnu/linux) |
> Thanks for the patch. I'm planning on giving Dave Airlie's series a
> try for 0.12.0. I'm pretty comfortable with those patches (since a
> few of them are mine :-)). I also don't think vmware-vga is going to
> be reliable without them so I don't think pulling in the one fix is
> good enough.
>
> His last patch has the same fix without the printf(). The printf is
> probably something to avoid since a malicious guest could create a
> storm of them. Since libvirt logs stderr by default, the result could
> be pretty nasty.
Fair enough... I just saw Dave's patches go by, and I guess we
independently fixed the cursor size thing at right around the same time.
How about the following, without the fprintf but with paranoid checks
(since a malicious guest could send a bad DEFINE_CURSOR and do who knows
what with the buffer overrun, which is even worse than spamming logs ;)
====
QEMU crashes with vmware_vga when running a Linux guest with the latest
X.org vmware video driver if QEMU is using SDL for video output. In
this case, vmware_vga advertises cursor acceleration to the guest, and
the crash comes when the guest does a DEFINE_CURSOR command with a 64x64
32bpp cursor. This request overruns the image[] array in struct
vmsvga_cursor_definition_s and QEMU ends up segfaulting because of
memory corruption caused by writing past the end of the array.
Fix this by enlarging the image[] array to be able to hold 4096 32-bit
pixels so we don't fail for the case of 64*64*32bpp, and also add error
checking to avoid a crash if an even bigger request is sent by a guest.
Signed-off-by: Roland Dreier <address@hidden>
---
hw/vmware_vga.c | 9 ++++++++-
1 files changed, 8 insertions(+), 1 deletions(-)
diff --git a/hw/vmware_vga.c b/hw/vmware_vga.c
index f3e3749..75d90f2 100644
--- a/hw/vmware_vga.c
+++ b/hw/vmware_vga.c
@@ -462,7 +462,7 @@ struct vmsvga_cursor_definition_s {
int hot_x;
int hot_y;
uint32_t mask[1024];
- uint32_t image[1024];
+ uint32_t image[4096]; /* allow for 64x64 32bpp cursor */
};
#define SVGA_BITMAP_SIZE(w, h) ((((w) + 31) >> 5) * (h))
@@ -557,6 +557,13 @@ static void vmsvga_fifo_run(struct vmsvga_state_s *s)
cursor.height = y = vmsvga_fifo_read(s);
vmsvga_fifo_read(s);
cursor.bpp = vmsvga_fifo_read(s);
+
+ if (SVGA_BITMAP_SIZE(x, y) > sizeof cursor.mask ||
+ SVGA_PIXMAP_SIZE(x, y, cursor.bpp) > sizeof cursor.image) {
+ args = SVGA_BITMAP_SIZE(x, y) + SVGA_PIXMAP_SIZE(x, y,
cursor.bpp);
+ goto badcmd;
+ }
+
for (args = 0; args < SVGA_BITMAP_SIZE(x, y); args ++)
cursor.mask[args] = vmsvga_fifo_read_raw(s);
for (args = 0; args < SVGA_PIXMAP_SIZE(x, y, cursor.bpp); args ++)