| Message ID | 20230627130231.1614896-23-marcandre.lureau@redhat.com |
|---|---|
| State | New |
| Headers | show |
| Series | [PULL,01/33] ui: return NULL when getting cursor without a console | expand |
On 230627 1502, marcandre.lureau@redhat.com wrote: > From: Marc-André Lureau <marcandre.lureau@redhat.com> > > Allocate pixman bits for scanouts with qemu_win32_map_alloc() so we can > set a shareable handle on the associated display surface. > > Note: when bits are provided to pixman_image_create_bits(), you must also give > the rowstride (the argument is ignored when bits is NULL) > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> > Message-Id: <20230606115658.677673-11-marcandre.lureau@redhat.com> > --- > include/hw/virtio/virtio-gpu.h | 3 +++ > hw/display/virtio-gpu.c | 46 +++++++++++++++++++++++++++++++--- > 2 files changed, 46 insertions(+), 3 deletions(-) > > diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h > index 2e28507efe..7a5f8056ea 100644 > --- a/include/hw/virtio/virtio-gpu.h > +++ b/include/hw/virtio/virtio-gpu.h > @@ -48,6 +48,9 @@ struct virtio_gpu_simple_resource { > unsigned int iov_cnt; > uint32_t scanout_bitmask; > pixman_image_t *image; > +#ifdef WIN32 > + HANDLE handle; > +#endif > uint64_t hostmem; > > uint64_t blob_size; > diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c > index 1f8a5b16c6..347e17d490 100644 > --- a/hw/display/virtio-gpu.c > +++ b/hw/display/virtio-gpu.c > @@ -258,6 +258,16 @@ static uint32_t calc_image_hostmem(pixman_format_code_t pformat, > return height * stride; > } > > +#ifdef WIN32 > +static void > +win32_pixman_image_destroy(pixman_image_t *image, void *data) > +{ > + HANDLE handle = data; > + > + qemu_win32_map_free(pixman_image_get_data(image), handle, &error_warn); > +} > +#endif > + > static void virtio_gpu_resource_create_2d(VirtIOGPU *g, > struct virtio_gpu_ctrl_command *cmd) > { > @@ -304,12 +314,27 @@ static void virtio_gpu_resource_create_2d(VirtIOGPU *g, > > res->hostmem = calc_image_hostmem(pformat, c2d.width, c2d.height); > if (res->hostmem + g->hostmem < g->conf_max_hostmem) { > + void *bits = NULL; > +#ifdef WIN32 > + bits = qemu_win32_map_alloc(res->hostmem, &res->handle, &error_warn); > + if (!bits) { > + goto end; > + } > +#endif > res->image = pixman_image_create_bits(pformat, > c2d.width, > c2d.height, > - NULL, 0); > + bits, res->hostmem / c2d.height); Hello, This may lead to FPE when c2d.height is 0. https://gitlab.com/qemu-project/qemu/-/issues/1744 -Alex
On Tue, 27 Jun 2023 at 14:07, <marcandre.lureau@redhat.com> wrote: > > From: Marc-André Lureau <marcandre.lureau@redhat.com> > > Allocate pixman bits for scanouts with qemu_win32_map_alloc() so we can > set a shareable handle on the associated display surface. > > Note: when bits are provided to pixman_image_create_bits(), you must also give > the rowstride (the argument is ignored when bits is NULL) > > Signed-off-by: Marc-André Lureau <marcandre.lureau@redhat.com> > Message-Id: <20230606115658.677673-11-marcandre.lureau@redhat.com> Hi; Coverity notes (CID 1516557) that this introduces a possible division-by-zero (different from the one Alex's fuzzer found): > @@ -1252,15 +1281,23 @@ static int virtio_gpu_load(QEMUFile *f, void *opaque, size_t size, > g_free(res); > return -EINVAL; > } > + > + res->hostmem = calc_image_hostmem(pformat, res->width, res->height); > +#ifdef WIN32 > + bits = qemu_win32_map_alloc(res->hostmem, &res->handle, &error_warn); > + if (!bits) { > + g_free(res); > + return -EINVAL; > + } > +#endif > res->image = pixman_image_create_bits(pformat, > res->width, res->height, > - NULL, 0); > + bits, res->hostmem / res->height); In this function we've just pulled res->height out of the incoming migration stream, and we haven't done any sanity checking on it. So it might be 0, in which case this division will divide by zero and fall over. thanks -- PMM
diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h index 2e28507efe..7a5f8056ea 100644 --- a/include/hw/virtio/virtio-gpu.h +++ b/include/hw/virtio/virtio-gpu.h @@ -48,6 +48,9 @@ struct virtio_gpu_simple_resource { unsigned int iov_cnt; uint32_t scanout_bitmask; pixman_image_t *image; +#ifdef WIN32 + HANDLE handle; +#endif uint64_t hostmem; uint64_t blob_size; diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c index 1f8a5b16c6..347e17d490 100644 --- a/hw/display/virtio-gpu.c +++ b/hw/display/virtio-gpu.c @@ -258,6 +258,16 @@ static uint32_t calc_image_hostmem(pixman_format_code_t pformat, return height * stride; } +#ifdef WIN32 +static void +win32_pixman_image_destroy(pixman_image_t *image, void *data) +{ + HANDLE handle = data; + + qemu_win32_map_free(pixman_image_get_data(image), handle, &error_warn); +} +#endif + static void virtio_gpu_resource_create_2d(VirtIOGPU *g, struct virtio_gpu_ctrl_command *cmd) { @@ -304,12 +314,27 @@ static void virtio_gpu_resource_create_2d(VirtIOGPU *g, res->hostmem = calc_image_hostmem(pformat, c2d.width, c2d.height); if (res->hostmem + g->hostmem < g->conf_max_hostmem) { + void *bits = NULL; +#ifdef WIN32 + bits = qemu_win32_map_alloc(res->hostmem, &res->handle, &error_warn); + if (!bits) { + goto end; + } +#endif res->image = pixman_image_create_bits(pformat, c2d.width, c2d.height, - NULL, 0); + bits, res->hostmem / c2d.height); +#ifdef WIN32 + if (res->image) { + pixman_image_set_destroy_function(res->image, win32_pixman_image_destroy, res->handle); + } +#endif } +#ifdef WIN32 +end: +#endif if (!res->image) { qemu_log_mask(LOG_GUEST_ERROR, "%s: resource creation failed %d %d %d\n", @@ -685,6 +710,9 @@ static void virtio_gpu_do_set_scanout(VirtIOGPU *g, *error = VIRTIO_GPU_RESP_ERR_UNSPEC; return; } +#ifdef WIN32 + qemu_displaysurface_win32_set_handle(scanout->ds, res->handle, fb->offset); +#endif pixman_image_unref(rect); dpy_gfx_replace_surface(g->parent_obj.scanout[scanout_id].con, @@ -1228,6 +1256,7 @@ static int virtio_gpu_load(QEMUFile *f, void *opaque, size_t size, struct virtio_gpu_simple_resource *res; struct virtio_gpu_scanout *scanout; uint32_t resource_id, pformat; + void *bits = NULL; int i; g->hostmem = 0; @@ -1252,15 +1281,23 @@ static int virtio_gpu_load(QEMUFile *f, void *opaque, size_t size, g_free(res); return -EINVAL; } + + res->hostmem = calc_image_hostmem(pformat, res->width, res->height); +#ifdef WIN32 + bits = qemu_win32_map_alloc(res->hostmem, &res->handle, &error_warn); + if (!bits) { + g_free(res); + return -EINVAL; + } +#endif res->image = pixman_image_create_bits(pformat, res->width, res->height, - NULL, 0); + bits, res->hostmem / res->height); if (!res->image) { g_free(res); return -EINVAL; } - res->hostmem = calc_image_hostmem(pformat, res->width, res->height); res->addrs = g_new(uint64_t, res->iov_cnt); res->iov = g_new(struct iovec, res->iov_cnt); @@ -1321,6 +1358,9 @@ static int virtio_gpu_load(QEMUFile *f, void *opaque, size_t size, if (!scanout->ds) { return -EINVAL; } +#ifdef WIN32 + qemu_displaysurface_win32_set_handle(scanout->ds, res->handle, 0); +#endif dpy_gfx_replace_surface(scanout->con, scanout->ds); dpy_gfx_update_full(scanout->con);