Re: [Qemu-devel] [PULL 5/9] contrib: add vhost-user-gpu

[Peter Maydell]

On Wed, 29 May 2019 at 05:42, Gerd Hoffmann <address@hidden> wrote:
>
> From: Marc-André Lureau <address@hidden>
>
> Add a vhost-user gpu backend, based on virtio-gpu/3d device. It is
> associated with a vhost-user-gpu device.
>
> Various TODO and nice to have items:
> - multi-head support
> - crash & resume handling
> - accelerated rendering/display that avoids the waiting round trips
> - edid support
>
> Signed-off-by: Marc-André Lureau <address@hidden>
> Message-id: address@hidden
> Signed-off-by: Gerd Hoffmann <address@hidden>

Hi; the latest coverity run has spotted a mismatch of
memory allocate/free, where memory allocated with malloc()
is freed with g_free():

> +static void
> +vg_handle_cursor(VuDev *dev, int qidx)
> +{
> +    VuGpu *g = container_of(dev, VuGpu, dev.parent);
> +    VuVirtq *vq = vu_get_queue(dev, qidx);
> +    VuVirtqElement *elem;
> +    size_t len;
> +    struct virtio_gpu_update_cursor cursor;
> +
> +    for (;;) {
> +        elem = vu_queue_pop(dev, vq, sizeof(VuVirtqElement));

vu_queue_pop() returns memory that must be freed with free()
(as documented in its API doc-comment; it calls vu_queue_map_desc()
which calls virtqueue_alloc_element() which calls malloc())...

> +        if (!elem) {
> +            break;
> +        }
> +        g_debug("cursor out:%d in:%d\n", elem->out_num, elem->in_num);
> +
> +        len = iov_to_buf(elem->out_sg, elem->out_num,
> +                         0, &cursor, sizeof(cursor));
> +        if (len != sizeof(cursor)) {
> +            g_warning("%s: cursor size incorrect %zu vs %zu\n",
> +                      __func__, len, sizeof(cursor));
> +        } else {
> +            virtio_gpu_bswap_32(&cursor, sizeof(cursor));
> +            vg_process_cursor_cmd(g, &cursor);
> +        }
> +        vu_queue_push(dev, vq, elem, 0);
> +        vu_queue_notify(dev, vq);
> +        g_free(elem);

...but here we free it with g_free(), not free().

Coverity spotted this as CID 1421887. The use of vu_queue_pop()
in vg_handle_ctrl() also seem to have this issue, though Coverity
hasn't caught that one.

Would somebody like to write a patch?

thanks
-- PMM