[Qemu-devel] [PULL 0/5] virtio-gpu: misc bugfixes.

[Qemu-devel] [PULL 0/5] virtio-gpu: misc bugfixes.

[Gerd Hoffmann]

  Hi,

New year, new pull request.  Here comes the vga patch queue with
a collection of virtio-gpu fixes.

please pull,
  Gerd

The following changes since commit 6a928d25b6d8bc3729c3d28326c6db13b9481059:

  Update version for v2.8.0-rc4 release (2016-12-15 07:36:03 +0000)

are available in the git repository at:

  git://git.kraxel.org/qemu tags/pull-vga-20170103-1

for you to fetch changes up to 204f01b30975923c64006f8067f0937b91eea68b:

  virtio-gpu: fix memory leak in resource attach backing (2017-01-03 15:47:21 +0100)

----------------------------------------------------------------
virtio-gpu: misc bugfixes.

----------------------------------------------------------------
Gerd Hoffmann (1):
      virtio-gpu: track and limit host memory allocations

Li Qiang (3):
      virtio-gpu: call cleanup mapping function in resource destroy
      virtio-gpu-3d: fix memory leak in resource attach backing
      virtio-gpu: fix memory leak in resource attach backing

Prasad J Pandit (1):
      display: virtio-gpu-3d: check virgl capabilities max_size

 hw/display/virtio-gpu-3d.c     | 13 ++++++++++---
 hw/display/virtio-gpu.c        | 24 ++++++++++++++++++++----
 include/hw/virtio/virtio-gpu.h |  3 +++
 3 files changed, 33 insertions(+), 7 deletions(-)

[Qemu-devel] [PULL 1/5] display: virtio-gpu-3d: check virgl capabilities max_size

[Gerd Hoffmann]

From: Prasad J Pandit <pjp@fedoraproject.org>

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 <zhenhaohong@gmail.com>
Signed-off-by: Prasad J Pandit <pjp@fedoraproject.org>
Message-id: 20161214070156.23368-1-ppandit@redhat.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 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 23f39de..e29f099 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);
+    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,
-- 
1.8.3.1

[Qemu-devel] [PULL 2/5] virtio-gpu: track and limit host memory allocations

[Gerd Hoffmann]

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 <marcandre.lureau@redhat.com>
Cc: Dave Airlie <airlied@redhat.com>
Cc: 李强 <liqiang6-s@360.cn>
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
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 5f32e1a..ed2b6d3 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 20d1cd6..f3a98a3 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;
 
-- 
1.8.3.1

[Qemu-devel] [PULL 3/5] virtio-gpu: call cleanup mapping function in resource destroy

[Gerd Hoffmann]

From: Li Qiang <liq3ea@gmail.com>

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 <liq3ea@gmail.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-id: 1480386565-10077-1-git-send-email-liq3ea@gmail.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 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 ed2b6d3..6a26258 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 <virglrenderer.h>
 #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);
-- 
1.8.3.1

[Qemu-devel] [PULL 4/5] virtio-gpu-3d: fix memory leak in resource attach backing

[Gerd Hoffmann]

From: Li Qiang <liq3ea@gmail.com>

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 <liq3ea@gmail.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-id: 1482999086-59795-1-git-send-email-liq3ea@gmail.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 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 e29f099..b13ced3 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,
-- 
1.8.3.1

[Qemu-devel] [PULL 5/5] virtio-gpu: fix memory leak in resource attach backing

[Gerd Hoffmann]

From: Li Qiang <liq3ea@gmail.com>

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 <liq3ea@gmail.com>
Message-id: 1483003721-65360-1-git-send-email-liq3ea@gmail.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 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 6a26258..ca88cf4 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;
-- 
1.8.3.1

Re: [Qemu-devel] [PULL 0/5] virtio-gpu: misc bugfixes.

[Peter Maydell]

On 3 January 2017 at 14:52, Gerd Hoffmann <kraxel@redhat.com> wrote:
>   Hi,
>
> New year, new pull request.  Here comes the vga patch queue with
> a collection of virtio-gpu fixes.
>
> please pull,
>   Gerd
>
> The following changes since commit 6a928d25b6d8bc3729c3d28326c6db13b9481059:
>
>   Update version for v2.8.0-rc4 release (2016-12-15 07:36:03 +0000)
>
> are available in the git repository at:
>
>   git://git.kraxel.org/qemu tags/pull-vga-20170103-1
>
> for you to fetch changes up to 204f01b30975923c64006f8067f0937b91eea68b:
>
>   virtio-gpu: fix memory leak in resource attach backing (2017-01-03 15:47:21 +0100)
>
> ----------------------------------------------------------------
> virtio-gpu: misc bugfixes.
>
Applied, thanks.

-- PMM