[Qemu-devel] [PULL 0/5] vga patch queue

[Qemu-devel] [PULL 0/5] vga patch queue

[Gerd Hoffmann]

  Hi,

Here comes the vga patch queue, with more bugfixes, one missed the
previously pull by mistake, the other ones are revent posts on the list.

please pull,
  Gerd

The following changes since commit b44486dfb9447c88e4b216e730adcc780190852c:

  Merge remote-tracking branch 'remotes/kraxel/tags/pull-ui-20170110-1' into staging (2017-01-10 14:52:34 +0000)

are available in the git repository at:


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

for you to fetch changes up to a2056e09b02745e5d000351a8a7938fa8a292ba7:

  virtio-gpu: tag as not hotpluggable (2017-01-11 09:19:05 +0100)

----------------------------------------------------------------
vga: fixes for virtio-gpu and cirrus.

----------------------------------------------------------------
Bruce Rogers (1):
      display: cirrus: ignore source pitch value as needed in blit_is_unsafe

Gerd Hoffmann (1):
      virtio-gpu: tag as not hotpluggable

Li Qiang (1):
      virtio-gpu: fix information leak in capset get dispatch

Peter Maydell (2):
      virtio-gpu: Recalculate VirtIOGPU::hostmem on VM load
      virtio-gpu: Fix memory leak in virtio_gpu_load()

 hw/display/cirrus_vga.c    | 11 +++++++----
 hw/display/virtio-gpu-3d.c |  2 +-
 hw/display/virtio-gpu.c    | 18 ++++++++++++++++++
 3 files changed, 26 insertions(+), 5 deletions(-)

[Qemu-devel] [PULL 1/5] virtio-gpu: fix information leak in capset get dispatch

[Gerd Hoffmann]

From: Li Qiang <liqiang6-s@360.cn>

In virgl_cmd_get_capset function, it uses g_malloc to allocate
a response struct to the guest. As the 'resp'struct hasn't been full
initialized it will lead the 'resp->padding' field to the guest.
Use g_malloc0 to avoid this.

Signed-off-by: Li Qiang <liqiang6-s@360.cn>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Message-id: 58188cae.4a6ec20a.3d2d1.aff2@mx.google.com

[ kraxel: resolved conflict ]

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/display/virtio-gpu-3d.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/hw/display/virtio-gpu-3d.c b/hw/display/virtio-gpu-3d.c
index b13ced3..f96a0c2 100644
--- a/hw/display/virtio-gpu-3d.c
+++ b/hw/display/virtio-gpu-3d.c
@@ -379,7 +379,7 @@ static void virgl_cmd_get_capset(VirtIOGPU *g,
         return;
     }
 
-    resp = g_malloc(sizeof(*resp) + max_size);
+    resp = g_malloc0(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] display: cirrus: ignore source pitch value as needed in blit_is_unsafe

[Gerd Hoffmann]

From: Bruce Rogers <brogers@suse.com>

Commit 4299b90 added a check which is too broad, given that the source
pitch value is not required to be initialized for solid fill operations.
This patch refines the blit_is_unsafe() check to ignore source pitch in
that case. After applying the above commit as a security patch, we
noticed the SLES 11 SP4 guest gui failed to initialize properly.

Signed-off-by: Bruce Rogers <brogers@suse.com>
Message-id: 20170109203520.5619-1-brogers@suse.com
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/display/cirrus_vga.c | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/hw/display/cirrus_vga.c b/hw/display/cirrus_vga.c
index bdb092e..379910d 100644
--- a/hw/display/cirrus_vga.c
+++ b/hw/display/cirrus_vga.c
@@ -294,7 +294,7 @@ static bool blit_region_is_unsafe(struct CirrusVGAState *s,
     return false;
 }
 
-static bool blit_is_unsafe(struct CirrusVGAState *s)
+static bool blit_is_unsafe(struct CirrusVGAState *s, bool dst_only)
 {
     /* should be the case, see cirrus_bitblt_start */
     assert(s->cirrus_blt_width > 0);
@@ -308,6 +308,9 @@ static bool blit_is_unsafe(struct CirrusVGAState *s)
                               s->cirrus_blt_dstaddr & s->cirrus_addr_mask)) {
         return true;
     }
+    if (dst_only) {
+        return false;
+    }
     if (blit_region_is_unsafe(s, s->cirrus_blt_srcpitch,
                               s->cirrus_blt_srcaddr & s->cirrus_addr_mask)) {
         return true;
@@ -673,7 +676,7 @@ static int cirrus_bitblt_common_patterncopy(CirrusVGAState * s,
 
     dst = s->vga.vram_ptr + (s->cirrus_blt_dstaddr & s->cirrus_addr_mask);
 
-    if (blit_is_unsafe(s))
+    if (blit_is_unsafe(s, false))
         return 0;
 
     (*s->cirrus_rop) (s, dst, src,
@@ -691,7 +694,7 @@ static int cirrus_bitblt_solidfill(CirrusVGAState *s, int blt_rop)
 {
     cirrus_fill_t rop_func;
 
-    if (blit_is_unsafe(s)) {
+    if (blit_is_unsafe(s, true)) {
         return 0;
     }
     rop_func = cirrus_fill[rop_to_index[blt_rop]][s->cirrus_blt_pixelwidth - 1];
@@ -795,7 +798,7 @@ static int cirrus_do_copy(CirrusVGAState *s, int dst, int src, int w, int h)
 
 static int cirrus_bitblt_videotovideo_copy(CirrusVGAState * s)
 {
-    if (blit_is_unsafe(s))
+    if (blit_is_unsafe(s, false))
         return 0;
 
     return cirrus_do_copy(s, s->cirrus_blt_dstaddr - s->vga.start_addr,
-- 
1.8.3.1

[Qemu-devel] [PULL 3/5] virtio-gpu: Recalculate VirtIOGPU::hostmem on VM load

[Gerd Hoffmann]

From: Peter Maydell <peter.maydell@linaro.org>

The 'hostmem' field in VirtIOGPU is used to track the total memory
used in pixmaps so that we can impose a maximum limit on it.
However this field is neither migrated nor recalculated on
VM load, which means that after a migration it will be incorrectly
too low, which can allow the guest to use more pixmap memory
than it should. The per-resource hostmem fields are not filled
in either as we reallocate them in the load function.

Recalculate the memory used for each pixmap and the total memory
used as we reallocate the pixmaps in virtio_gpu_load().

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Message-id: 1483969123-14839-2-git-send-email-peter.maydell@linaro.org
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 ca88cf4..c3cf47e 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -1038,6 +1038,8 @@ static int virtio_gpu_load(QEMUFile *f, void *opaque, size_t size)
     uint32_t resource_id, pformat;
     int i;
 
+    g->hostmem = 0;
+
     resource_id = qemu_get_be32(f);
     while (resource_id != 0) {
         res = g_new0(struct virtio_gpu_simple_resource, 1);
@@ -1059,6 +1061,8 @@ static int virtio_gpu_load(QEMUFile *f, void *opaque, size_t size)
             return -EINVAL;
         }
 
+        res->hostmem = PIXMAN_FORMAT_BPP(pformat) * res->width * res->height;
+
         res->addrs = g_new(uint64_t, res->iov_cnt);
         res->iov = g_new(struct iovec, res->iov_cnt);
 
@@ -1081,6 +1085,7 @@ static int virtio_gpu_load(QEMUFile *f, void *opaque, size_t size)
         }
 
         QTAILQ_INSERT_HEAD(&g->reslist, res, next);
+        g->hostmem += res->hostmem;
 
         resource_id = qemu_get_be32(f);
     }
-- 
1.8.3.1

[Qemu-devel] [PULL 4/5] virtio-gpu: Fix memory leak in virtio_gpu_load()

[Gerd Hoffmann]

From: Peter Maydell <peter.maydell@linaro.org>

Coverity points out that if we fail in the "creating resources"
loop in virtio_gpu_load() we will leak various resources (CID 1356431).
Failing a VM load is going to leave the simulation in a complete mess,
but we can tidy up to the point that a full system reset should
get us back to sanity.

Signed-off-by: Peter Maydell <peter.maydell@linaro.org>
Message-id: 1483969123-14839-3-git-send-email-peter.maydell@linaro.org
Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
---
 hw/display/virtio-gpu.c | 12 ++++++++++++
 1 file changed, 12 insertions(+)

diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index c3cf47e..cef736c 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -1052,12 +1052,14 @@ static int virtio_gpu_load(QEMUFile *f, void *opaque, size_t size)
         /* allocate */
         pformat = get_pixman_format(res->format);
         if (!pformat) {
+            g_free(res);
             return -EINVAL;
         }
         res->image = pixman_image_create_bits(pformat,
                                               res->width, res->height,
                                               NULL, 0);
         if (!res->image) {
+            g_free(res);
             return -EINVAL;
         }
 
@@ -1080,6 +1082,16 @@ static int virtio_gpu_load(QEMUFile *f, void *opaque, size_t size)
             res->iov[i].iov_base =
                 cpu_physical_memory_map(res->addrs[i], &len, 1);
             if (!res->iov[i].iov_base || len != res->iov[i].iov_len) {
+                /* Clean up the half-a-mapping we just created... */
+                if (res->iov[i].iov_base) {
+                    cpu_physical_memory_unmap(res->iov[i].iov_base,
+                                              len, 0, 0);
+                }
+                /* ...and the mappings for previous loop iterations */
+                res->iov_cnt = i;
+                virtio_gpu_cleanup_mapping(res);
+                pixman_image_unref(res->image);
+                g_free(res);
                 return -EINVAL;
             }
         }
-- 
1.8.3.1

[Qemu-devel] [PULL 5/5] virtio-gpu: tag as not hotpluggable

[Gerd Hoffmann]

qemu can't hotplug display devices.

Signed-off-by: Gerd Hoffmann <kraxel@redhat.com>
Reviewed-by: Michael S. Tsirkin <mst@redhat.com>
Message-id: 1483970138-20360-1-git-send-email-kraxel@redhat.com
---
 hw/display/virtio-gpu.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index cef736c..7a15c61 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -1299,6 +1299,7 @@ static void virtio_gpu_class_init(ObjectClass *klass, void *data)
 
     dc->props = virtio_gpu_properties;
     dc->vmsd = &vmstate_virtio_gpu;
+    dc->hotpluggable = false;
 }
 
 static const TypeInfo virtio_gpu_info = {
-- 
1.8.3.1

Re: [Qemu-devel] [PULL 0/5] vga patch queue

[Peter Maydell]

On 11 January 2017 at 10:28, Gerd Hoffmann <kraxel@redhat.com> wrote:
>   Hi,
>
> Here comes the vga patch queue, with more bugfixes, one missed the
> previously pull by mistake, the other ones are revent posts on the list.
>
> please pull,
>   Gerd
>
> The following changes since commit b44486dfb9447c88e4b216e730adcc780190852c:
>
>   Merge remote-tracking branch 'remotes/kraxel/tags/pull-ui-20170110-1' into staging (2017-01-10 14:52:34 +0000)
>
> are available in the git repository at:
>
>
>   git://git.kraxel.org/qemu tags/pull-vga-20170111-1
>
> for you to fetch changes up to a2056e09b02745e5d000351a8a7938fa8a292ba7:
>
>   virtio-gpu: tag as not hotpluggable (2017-01-11 09:19:05 +0100)
>
> ----------------------------------------------------------------
> vga: fixes for virtio-gpu and cirrus.
>
> ----------------------------------------------------------------

Applied, thanks.

-- PMM