From abd7f08b2353f43274b785db8c7224f082ef4d31 Mon Sep 17 00:00:00 2001 From: Prasad J Pandit Date: Wed, 14 Dec 2016 12:31:56 +0530 Subject: display: virtio-gpu-3d: check virgl capabilities max_size Virtio GPU device while processing 'VIRTIO_GPU_CMD_GET_CAPSET' command, retrieves the maximum capabilities size to fill in the response object. It continues to fill in capabilities even if retrieved 'max_size' is zero(0), thus resulting in OOB access. Add check to avoid it. Reported-by: Zhenhao Hong Signed-off-by: Prasad J Pandit Message-id: 20161214070156.23368-1-ppandit@redhat.com Signed-off-by: Gerd Hoffmann --- hw/display/virtio-gpu-3d.c | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/hw/display/virtio-gpu-3d.c b/hw/display/virtio-gpu-3d.c index 23f39de94d..e29f099bd5 100644 --- a/hw/display/virtio-gpu-3d.c +++ b/hw/display/virtio-gpu-3d.c @@ -371,8 +371,12 @@ static void virgl_cmd_get_capset(VirtIOGPU *g, virgl_renderer_get_cap_set(gc.capset_id, &max_ver, &max_size); - resp = g_malloc(sizeof(*resp) + max_size); + if (!max_size) { + cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_PARAMETER; + return; + } + resp = g_malloc(sizeof(*resp) + max_size); resp->hdr.type = VIRTIO_GPU_RESP_OK_CAPSET; virgl_renderer_fill_caps(gc.capset_id, gc.capset_version, -- cgit v1.2.3 From 9b7621bca2f70dc1a9815d50f05261296a8ae932 Mon Sep 17 00:00:00 2001 From: Gerd Hoffmann Date: Tue, 29 Nov 2016 13:42:36 +0100 Subject: virtio-gpu: track and limit host memory allocations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This patch makes virtio-gpu track host memory allocations for ressources and applies a limit (configurable 256M by default). When exceeding the limit virtio-gpu throws VIRTIO_GPU_RESP_ERR_OUT_OF_MEMORY errors (like it already does today when pixman image allocations fail). This patch covers 2d mode only. For 3d mode we have to figure how we are going to handle this best. qemu doesn't track resources in case virglrenderer is used, so I guess we should extend virglrenderer to allow setting a limit, then let qemu set the limit and catch virgl_renderer_resource_create failures. Cc: Marc-André Lureau Cc: Dave Airlie Cc: 李强 Signed-off-by: Gerd Hoffmann Message-id: 1480423356-22255-1-git-send-email-kraxel@redhat.com --- hw/display/virtio-gpu.c | 16 ++++++++++++---- include/hw/virtio/virtio-gpu.h | 3 +++ 2 files changed, 15 insertions(+), 4 deletions(-) diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c index 5f32e1aae9..ed2b6d3deb 100644 --- a/hw/display/virtio-gpu.c +++ b/hw/display/virtio-gpu.c @@ -338,10 +338,14 @@ static void virtio_gpu_resource_create_2d(VirtIOGPU *g, cmd->error = VIRTIO_GPU_RESP_ERR_INVALID_PARAMETER; return; } - res->image = pixman_image_create_bits(pformat, - c2d.width, - c2d.height, - NULL, 0); + + res->hostmem = PIXMAN_FORMAT_BPP(pformat) * c2d.width * c2d.height; + if (res->hostmem + g->hostmem < g->conf.max_hostmem) { + res->image = pixman_image_create_bits(pformat, + c2d.width, + c2d.height, + NULL, 0); + } if (!res->image) { qemu_log_mask(LOG_GUEST_ERROR, @@ -353,6 +357,7 @@ static void virtio_gpu_resource_create_2d(VirtIOGPU *g, } QTAILQ_INSERT_HEAD(&g->reslist, res, next); + g->hostmem += res->hostmem; } static void virtio_gpu_resource_destroy(VirtIOGPU *g, @@ -360,6 +365,7 @@ static void virtio_gpu_resource_destroy(VirtIOGPU *g, { pixman_image_unref(res->image); QTAILQ_REMOVE(&g->reslist, res, next); + g->hostmem -= res->hostmem; g_free(res); } @@ -1241,6 +1247,8 @@ static const VMStateDescription vmstate_virtio_gpu = { static Property virtio_gpu_properties[] = { DEFINE_PROP_UINT32("max_outputs", VirtIOGPU, conf.max_outputs, 1), + DEFINE_PROP_SIZE("max_hostmem", VirtIOGPU, conf.max_hostmem, + 256 * 1024 * 1024), #ifdef CONFIG_VIRGL DEFINE_PROP_BIT("virgl", VirtIOGPU, conf.flags, VIRTIO_GPU_FLAG_VIRGL_ENABLED, true), diff --git a/include/hw/virtio/virtio-gpu.h b/include/hw/virtio/virtio-gpu.h index 20d1cd683a..f3a98a3261 100644 --- a/include/hw/virtio/virtio-gpu.h +++ b/include/hw/virtio/virtio-gpu.h @@ -38,6 +38,7 @@ struct virtio_gpu_simple_resource { unsigned int iov_cnt; uint32_t scanout_bitmask; pixman_image_t *image; + uint64_t hostmem; QTAILQ_ENTRY(virtio_gpu_simple_resource) next; }; @@ -68,6 +69,7 @@ enum virtio_gpu_conf_flags { (_cfg.flags & (1 << VIRTIO_GPU_FLAG_STATS_ENABLED)) struct virtio_gpu_conf { + uint64_t max_hostmem; uint32_t max_outputs; uint32_t flags; }; @@ -103,6 +105,7 @@ typedef struct VirtIOGPU { struct virtio_gpu_requested_state req_state[VIRTIO_GPU_MAX_SCANOUTS]; struct virtio_gpu_conf conf; + uint64_t hostmem; int enabled_output_bitmask; struct virtio_gpu_config virtio_config; -- cgit v1.2.3 From b8e23926c568f2e963af39028b71c472e3023793 Mon Sep 17 00:00:00 2001 From: Li Qiang Date: Mon, 28 Nov 2016 21:29:25 -0500 Subject: virtio-gpu: call cleanup mapping function in resource destroy MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If the guest destroy the resource before detach banking, the 'iov' and 'addrs' field in resource is not freed thus leading memory leak issue. This patch avoid this. Signed-off-by: Li Qiang Reviewed-by: Marc-André Lureau Message-id: 1480386565-10077-1-git-send-email-liq3ea@gmail.com Signed-off-by: Gerd Hoffmann --- hw/display/virtio-gpu.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c index ed2b6d3deb..6a26258cac 100644 --- a/hw/display/virtio-gpu.c +++ b/hw/display/virtio-gpu.c @@ -28,6 +28,8 @@ static struct virtio_gpu_simple_resource* virtio_gpu_find_resource(VirtIOGPU *g, uint32_t resource_id); +static void virtio_gpu_cleanup_mapping(struct virtio_gpu_simple_resource *res); + #ifdef CONFIG_VIRGL #include #define VIRGL(_g, _virgl, _simple, ...) \ @@ -364,6 +366,7 @@ static void virtio_gpu_resource_destroy(VirtIOGPU *g, struct virtio_gpu_simple_resource *res) { pixman_image_unref(res->image); + virtio_gpu_cleanup_mapping(res); QTAILQ_REMOVE(&g->reslist, res, next); g->hostmem -= res->hostmem; g_free(res); -- cgit v1.2.3 From 33243031dad02d161225ba99d782616da133f689 Mon Sep 17 00:00:00 2001 From: Li Qiang Date: Thu, 29 Dec 2016 03:11:26 -0500 Subject: virtio-gpu-3d: fix memory leak in resource attach backing MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If the virgl_renderer_resource_attach_iov function fails the 'res_iovs' will be leaked. Add check of the return value to free the 'res_iovs' when failing. Signed-off-by: Li Qiang Reviewed-by: Marc-André Lureau Message-id: 1482999086-59795-1-git-send-email-liq3ea@gmail.com Signed-off-by: Gerd Hoffmann --- hw/display/virtio-gpu-3d.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/hw/display/virtio-gpu-3d.c b/hw/display/virtio-gpu-3d.c index e29f099bd5..b13ced38fa 100644 --- a/hw/display/virtio-gpu-3d.c +++ b/hw/display/virtio-gpu-3d.c @@ -291,8 +291,11 @@ static void virgl_resource_attach_backing(VirtIOGPU *g, return; } - virgl_renderer_resource_attach_iov(att_rb.resource_id, - res_iovs, att_rb.nr_entries); + ret = virgl_renderer_resource_attach_iov(att_rb.resource_id, + res_iovs, att_rb.nr_entries); + + if (ret != 0) + virtio_gpu_cleanup_mapping_iov(res_iovs, att_rb.nr_entries); } static void virgl_resource_detach_backing(VirtIOGPU *g, -- cgit v1.2.3 From 204f01b30975923c64006f8067f0937b91eea68b Mon Sep 17 00:00:00 2001 From: Li Qiang Date: Thu, 29 Dec 2016 04:28:41 -0500 Subject: virtio-gpu: fix memory leak in resource attach backing In the resource attach backing function, everytime it will allocate 'res->iov' thus can leading a memory leak. This patch avoid this. Signed-off-by: Li Qiang Message-id: 1483003721-65360-1-git-send-email-liq3ea@gmail.com Signed-off-by: Gerd Hoffmann --- hw/display/virtio-gpu.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c index 6a26258cac..ca88cf478d 100644 --- a/hw/display/virtio-gpu.c +++ b/hw/display/virtio-gpu.c @@ -714,6 +714,11 @@ virtio_gpu_resource_attach_backing(VirtIOGPU *g, return; } + if (res->iov) { + cmd->error = VIRTIO_GPU_RESP_ERR_UNSPEC; + return; + } + ret = virtio_gpu_create_mapping_iov(&ab, cmd, &res->addrs, &res->iov); if (ret != 0) { cmd->error = VIRTIO_GPU_RESP_ERR_UNSPEC; -- cgit v1.2.3