From patchwork Sun Dec 20 18:06:06 2009 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit Subject: Re: [PATCH to consider for 0.12] vmware_vga: Don't crash on too-big DEFINE_CURSOR command Date: Sun, 20 Dec 2009 08:06:06 -0000 From: Roland Dreier X-Patchwork-Id: 41512 Message-Id: To: Anthony Liguori Cc: Dave Airlie , qemu-devel@nongnu.org I see that you applied Dave's series, no worries on that, but I think we really want the following, since a clever enough malicious guest can probably turn the overrun into code execution in the host QEMU (though I've not tried writing an exploit or anything like that)... not sure if this merits the whole CVE process but it does look very much worth fixing, and probably the other guest commands want auditing too... === vmware_vga: Check cursor dimensions passed from guest to avoid buffer overflow Check that the cursor dimensions passed from the guest for the DEFINE_CURSOR command don't overflow the available space in the cursor.image[] or cursor.mask[] arrays before copying data from the guest into those arrays. Signed-off-by: Roland Dreier --- hw/vmware_vga.c | 7 +++++++ 1 files changed, 7 insertions(+), 0 deletions(-) diff --git a/hw/vmware_vga.c b/hw/vmware_vga.c index 7ab1c79..5e969ae 100644 --- a/hw/vmware_vga.c +++ b/hw/vmware_vga.c @@ -562,6 +562,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 ++)