> 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 <rola...@cisco.com> --- 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 ++)