On 09/05/14 11:33, Gerd Hoffmann wrote:
> On Fr, 2014-09-05 at 11:06 +0200, Laszlo Ersek wrote:
>>> > > Makes sense. I think it is easier to just multiply in 64bit, then
>> > check
>>> > > the result is small enougth (new patch attached).
>> >
>> > Okay, if you can guarantee that the product fits in uint64_t, then
>> > such
>> > a check would suffice.
>> >
>> > New patch has not been attached though :)
> Oops. Here we go.
>
> cheers,
> Gerd
>
>
> 0001-spice-make-sure-we-don-t-overflow-ssd-buf.patch
>
>
> From 33c5c3d1736fd577fc1279a1f3c50d2414e98fe3 Mon Sep 17 00:00:00 2001
> From: Gerd Hoffmann <kraxel@xxxxxxxxxx>
> Date: Wed, 3 Sep 2014 15:50:08 +0200
> Subject: [PATCH] spice: make sure we don't overflow ssd->buf
>
> Related spice-only bug. We have a fixed 16 MB buffer here, being
> presented to the spice-server as qxl video memory in case spice is
> used with a non-qxl card. It's also used with qxl in vga mode.
>
> When using display resolutions requiring more than 16 MB of memory we
> are going to overflow that buffer. In theory the guest can write,
> indirectly via spice-server. The spice-server clears the memory after
> setting a new video mode though, triggering a segfault in the overflow
> case, so qemu crashes before the guest has a chance to do something
> evil.
>
> Fix that by switching to dynamic allocation for the buffer.
>
> CVE-2014-3615
>
> Cc: qemu-stable@xxxxxxxxxx
> Cc: secalert@xxxxxxxxxx
> Cc: Laszlo Ersek <lersek@xxxxxxxxxx>
> Signed-off-by: Gerd Hoffmann <kraxel@xxxxxxxxxx>
> ---
> ui/spice-display.c | 20 +++++++++++++++-----
> 1 file changed, 15 insertions(+), 5 deletions(-)
>
> diff --git a/ui/spice-display.c b/ui/spice-display.c
> index 66e2578..def7b52 100644
> --- a/ui/spice-display.c
> +++ b/ui/spice-display.c
> @@ -334,11 +334,23 @@ void qemu_spice_create_host_memslot(SimpleSpiceDisplay *ssd)
> void qemu_spice_create_host_primary(SimpleSpiceDisplay *ssd)
> {
> QXLDevSurfaceCreate surface;
> + uint64_t surface_size;
>
> memset(&surface, 0, sizeof(surface));
>
> - dprint(1, "%s/%d: %dx%d\n", __func__, ssd->qxl.id,
> - surface_width(ssd->ds), surface_height(ssd->ds));
> + surface_size = (uint64_t) surface_width(ssd->ds) *
> + surface_height(ssd->ds) * 4;
> + assert(surface_size > 0);
> + assert(surface_size < INT_MAX);
> + if (ssd->bufsize < surface_size) {
> + ssd->bufsize = surface_size;
> + g_free(ssd->buf);
> + ssd->buf = g_malloc(ssd->bufsize);
> + }
> +
> + dprint(1, "%s/%d: %ux%u (size %" PRIu64 "/%d)\n", __func__, ssd->qxl.id,
> + surface_width(ssd->ds), surface_height(ssd->ds),
> + surface_size, ssd->bufsize);
>
> surface.format = SPICE_SURFACE_FMT_32_xRGB;
> surface.width = surface_width(ssd->ds);
> @@ -369,8 +381,6 @@ void qemu_spice_display_init_common(SimpleSpiceDisplay *ssd)
> if (ssd->num_surfaces == 0) {
> ssd->num_surfaces = 1024;
> }
> - ssd->bufsize = (16 * 1024 * 1024);
> - ssd->buf = g_malloc(ssd->bufsize);
> }
>
> /* display listener callbacks */
> @@ -495,7 +505,7 @@ static void interface_get_init_info(QXLInstance *sin, QXLDevInitInfo *info)
> info->num_memslots = NUM_MEMSLOTS;
> info->num_memslots_groups = NUM_MEMSLOTS_GROUPS;
> info->internal_groupslot_id = 0;
> - info->qxl_ram_size = ssd->bufsize;
> + info->qxl_ram_size = 16 * 1024 * 1024;
> info->n_surfaces = ssd->num_surfaces;
> }
>
> -- 1.8.3.1
>
Reviewed-by: Laszlo Ersek <lersek@xxxxxxxxxx>
_______________________________________________
Spice-devel mailing list
Spice-devel@xxxxxxxxxxxxxxxxxxxxx
http://lists.freedesktop.org/mailman/listinfo/spice-devel