qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH-for-5.0 0/3] virtio,vhost-gpu: Release memory returned by malloc() with free()
@ 2020-03-23 11:29 Philippe Mathieu-Daudé
  2020-03-23 11:29 ` [PATCH-for-5.0 1/3] vhost-user-gpu: Release memory returned by vu_queue_pop() " Philippe Mathieu-Daudé
                   ` (3 more replies)
  0 siblings, 4 replies; 13+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-03-23 11:29 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Maydell, Philippe Mathieu-Daudé,
	Gerd Hoffmann, Marc-André Lureau, Michael S. Tsirkin

Coverity reported a ALLOC_FREE_MISMATCH in vg_handle_cursor(),
because the memory returned by vu_queue_pop() is allocated with
malloc(). Fix it.

Similar error occurs with virtio. Document and fix.

Philippe Mathieu-Daudé (3):
  vhost-user-gpu: Release memory returned by vu_queue_pop() with free()
  virtio: Document virtqueue_pop()
  virtio-gpu: Release memory returned by virtqueue_pop() with free()

 include/hw/virtio/virtio.h              | 8 ++++++++
 contrib/vhost-user-gpu/vhost-user-gpu.c | 4 ++--
 contrib/vhost-user-gpu/virgl.c          | 2 +-
 hw/display/virtio-gpu-3d.c              | 2 +-
 hw/display/virtio-gpu.c                 | 8 ++++----
 5 files changed, 16 insertions(+), 8 deletions(-)

-- 
2.21.1



^ permalink raw reply	[flat|nested] 13+ messages in thread

* [PATCH-for-5.0 1/3] vhost-user-gpu: Release memory returned by vu_queue_pop() with free()
  2020-03-23 11:29 [PATCH-for-5.0 0/3] virtio,vhost-gpu: Release memory returned by malloc() with free() Philippe Mathieu-Daudé
@ 2020-03-23 11:29 ` Philippe Mathieu-Daudé
  2020-03-23 11:54   ` Marc-André Lureau
  2020-04-15 19:27   ` Peter Maydell
  2020-03-23 11:29 ` [PATCH-for-5.0 2/3] virtio: Document virtqueue_pop() Philippe Mathieu-Daudé
                   ` (2 subsequent siblings)
  3 siblings, 2 replies; 13+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-03-23 11:29 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Maydell, Michael S. Tsirkin, qemu-stable, Gerd Hoffmann,
	Marc-André Lureau, Philippe Mathieu-Daudé

vu_queue_pop() returns memory that must be freed with free().

Cc: qemu-stable@nongnu.org
Reported-by: Coverity (CID 1421887 ALLOC_FREE_MISMATCH)
Suggested-by: Peter Maydell <peter.maydell@linaro.org>
Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 contrib/vhost-user-gpu/vhost-user-gpu.c | 4 ++--
 contrib/vhost-user-gpu/virgl.c          | 2 +-
 2 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/contrib/vhost-user-gpu/vhost-user-gpu.c b/contrib/vhost-user-gpu/vhost-user-gpu.c
index b45d2019b4..a019d0a9ac 100644
--- a/contrib/vhost-user-gpu/vhost-user-gpu.c
+++ b/contrib/vhost-user-gpu/vhost-user-gpu.c
@@ -848,7 +848,7 @@ vg_handle_ctrl(VuDev *dev, int qidx)
             QTAILQ_INSERT_TAIL(&vg->fenceq, cmd, next);
             vg->inflight++;
         } else {
-            g_free(cmd);
+            free(cmd);
         }
     }
 }
@@ -939,7 +939,7 @@ vg_handle_cursor(VuDev *dev, int qidx)
         }
         vu_queue_push(dev, vq, elem, 0);
         vu_queue_notify(dev, vq);
-        g_free(elem);
+        free(elem);
     }
 }
 
diff --git a/contrib/vhost-user-gpu/virgl.c b/contrib/vhost-user-gpu/virgl.c
index 43413e29df..b0bc22c3c1 100644
--- a/contrib/vhost-user-gpu/virgl.c
+++ b/contrib/vhost-user-gpu/virgl.c
@@ -519,7 +519,7 @@ virgl_write_fence(void *opaque, uint32_t fence)
         g_debug("FENCE %" PRIu64, cmd->cmd_hdr.fence_id);
         vg_ctrl_response_nodata(g, cmd, VIRTIO_GPU_RESP_OK_NODATA);
         QTAILQ_REMOVE(&g->fenceq, cmd, next);
-        g_free(cmd);
+        free(cmd);
         g->inflight--;
     }
 }
-- 
2.21.1



^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH-for-5.0 2/3] virtio: Document virtqueue_pop()
  2020-03-23 11:29 [PATCH-for-5.0 0/3] virtio,vhost-gpu: Release memory returned by malloc() with free() Philippe Mathieu-Daudé
  2020-03-23 11:29 ` [PATCH-for-5.0 1/3] vhost-user-gpu: Release memory returned by vu_queue_pop() " Philippe Mathieu-Daudé
@ 2020-03-23 11:29 ` Philippe Mathieu-Daudé
  2020-03-23 11:54   ` Marc-André Lureau
  2020-03-23 11:29 ` [PATCH-for-5.0 3/3] virtio-gpu: Release memory returned by virtqueue_pop() with free() Philippe Mathieu-Daudé
  2020-03-23 13:41 ` [PATCH-for-5.0 0/3] virtio, vhost-gpu: Release memory returned by malloc() " Michael S. Tsirkin
  3 siblings, 1 reply; 13+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-03-23 11:29 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Maydell, Philippe Mathieu-Daudé,
	Gerd Hoffmann, Marc-André Lureau, Michael S. Tsirkin

Document that virtqueue_pop() returned memory must be released
with free().

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 include/hw/virtio/virtio.h | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
index b69d517496..c6e3bfc500 100644
--- a/include/hw/virtio/virtio.h
+++ b/include/hw/virtio/virtio.h
@@ -199,6 +199,14 @@ void virtqueue_fill(VirtQueue *vq, const VirtQueueElement *elem,
                     unsigned int len, unsigned int idx);
 
 void virtqueue_map(VirtIODevice *vdev, VirtQueueElement *elem);
+/**
+ * virtqueue_pop:
+ * @vq: a VirtQueue queue
+ * @sz: the size of struct to return (must be >= VirtQueueElement)
+ *
+ * Returns: a VirtQueueElement filled from the queue or NULL.
+ * The returned element must be free()-d by the caller.
+ */
 void *virtqueue_pop(VirtQueue *vq, size_t sz);
 unsigned int virtqueue_drop_all(VirtQueue *vq);
 void *qemu_get_virtqueue_element(VirtIODevice *vdev, QEMUFile *f, size_t sz);
-- 
2.21.1



^ permalink raw reply related	[flat|nested] 13+ messages in thread

* [PATCH-for-5.0 3/3] virtio-gpu: Release memory returned by virtqueue_pop() with free()
  2020-03-23 11:29 [PATCH-for-5.0 0/3] virtio,vhost-gpu: Release memory returned by malloc() with free() Philippe Mathieu-Daudé
  2020-03-23 11:29 ` [PATCH-for-5.0 1/3] vhost-user-gpu: Release memory returned by vu_queue_pop() " Philippe Mathieu-Daudé
  2020-03-23 11:29 ` [PATCH-for-5.0 2/3] virtio: Document virtqueue_pop() Philippe Mathieu-Daudé
@ 2020-03-23 11:29 ` Philippe Mathieu-Daudé
  2020-03-25 14:31   ` Marc-André Lureau
  2020-03-23 13:41 ` [PATCH-for-5.0 0/3] virtio, vhost-gpu: Release memory returned by malloc() " Michael S. Tsirkin
  3 siblings, 1 reply; 13+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-03-23 11:29 UTC (permalink / raw)
  To: qemu-devel
  Cc: Peter Maydell, Philippe Mathieu-Daudé,
	Gerd Hoffmann, Marc-André Lureau, Michael S. Tsirkin

As virtio_gpu_handle_ctrl() fills the cmdq calling virtqueue_pop(),
we need to release it with free() in virtio_gpu_reset().

virtio_gpu_handle_ctrl() allocates memory calling virtqueue_pop(),
release it in virtio_gpu_process_cmdq() with free().

virtio_gpu_process_cmdq() move commands from cmdq to fenceq, so
we also have to release them with free().

Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
---
 hw/display/virtio-gpu-3d.c | 2 +-
 hw/display/virtio-gpu.c    | 8 ++++----
 2 files changed, 5 insertions(+), 5 deletions(-)

diff --git a/hw/display/virtio-gpu-3d.c b/hw/display/virtio-gpu-3d.c
index 96621576c2..3a97d267e5 100644
--- a/hw/display/virtio-gpu-3d.c
+++ b/hw/display/virtio-gpu-3d.c
@@ -506,7 +506,7 @@ static void virgl_write_fence(void *opaque, uint32_t fence)
         trace_virtio_gpu_fence_resp(cmd->cmd_hdr.fence_id);
         virtio_gpu_ctrl_response_nodata(g, cmd, VIRTIO_GPU_RESP_OK_NODATA);
         QTAILQ_REMOVE(&g->fenceq, cmd, next);
-        g_free(cmd);
+        free(cmd);
         g->inflight--;
         if (virtio_gpu_stats_enabled(g->parent_obj.conf)) {
             fprintf(stderr, "inflight: %3d (-)\r", g->inflight);
diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
index 5f0dd7c150..f5fbb722ee 100644
--- a/hw/display/virtio-gpu.c
+++ b/hw/display/virtio-gpu.c
@@ -835,7 +835,7 @@ void virtio_gpu_process_cmdq(VirtIOGPU *g)
                 fprintf(stderr, "inflight: %3d (+)\r", g->inflight);
             }
         } else {
-            g_free(cmd);
+            free(cmd);
         }
     }
 }
@@ -921,7 +921,7 @@ static void virtio_gpu_handle_cursor(VirtIODevice *vdev, VirtQueue *vq)
         }
         virtqueue_push(vq, elem, 0);
         virtio_notify(vdev, vq);
-        g_free(elem);
+        free(elem);
     }
 }
 
@@ -1157,14 +1157,14 @@ static void virtio_gpu_reset(VirtIODevice *vdev)
     while (!QTAILQ_EMPTY(&g->cmdq)) {
         cmd = QTAILQ_FIRST(&g->cmdq);
         QTAILQ_REMOVE(&g->cmdq, cmd, next);
-        g_free(cmd);
+        free(cmd);
     }
 
     while (!QTAILQ_EMPTY(&g->fenceq)) {
         cmd = QTAILQ_FIRST(&g->fenceq);
         QTAILQ_REMOVE(&g->fenceq, cmd, next);
         g->inflight--;
-        g_free(cmd);
+        free(cmd);
     }
 
 #ifdef CONFIG_VIRGL
-- 
2.21.1



^ permalink raw reply related	[flat|nested] 13+ messages in thread

* Re: [PATCH-for-5.0 1/3] vhost-user-gpu: Release memory returned by vu_queue_pop() with free()
  2020-03-23 11:29 ` [PATCH-for-5.0 1/3] vhost-user-gpu: Release memory returned by vu_queue_pop() " Philippe Mathieu-Daudé
@ 2020-03-23 11:54   ` Marc-André Lureau
  2020-04-15 19:27   ` Peter Maydell
  1 sibling, 0 replies; 13+ messages in thread
From: Marc-André Lureau @ 2020-03-23 11:54 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Peter Maydell, qemu-stable, Michael S. Tsirkin, qemu-devel,
	Gerd Hoffmann

On Mon, Mar 23, 2020 at 12:29 PM Philippe Mathieu-Daudé
<philmd@redhat.com> wrote:
>
> vu_queue_pop() returns memory that must be freed with free().
>
> Cc: qemu-stable@nongnu.org
> Reported-by: Coverity (CID 1421887 ALLOC_FREE_MISMATCH)
> Suggested-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>


> ---
>  contrib/vhost-user-gpu/vhost-user-gpu.c | 4 ++--
>  contrib/vhost-user-gpu/virgl.c          | 2 +-
>  2 files changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/contrib/vhost-user-gpu/vhost-user-gpu.c b/contrib/vhost-user-gpu/vhost-user-gpu.c
> index b45d2019b4..a019d0a9ac 100644
> --- a/contrib/vhost-user-gpu/vhost-user-gpu.c
> +++ b/contrib/vhost-user-gpu/vhost-user-gpu.c
> @@ -848,7 +848,7 @@ vg_handle_ctrl(VuDev *dev, int qidx)
>              QTAILQ_INSERT_TAIL(&vg->fenceq, cmd, next);
>              vg->inflight++;
>          } else {
> -            g_free(cmd);
> +            free(cmd);
>          }
>      }
>  }
> @@ -939,7 +939,7 @@ vg_handle_cursor(VuDev *dev, int qidx)
>          }
>          vu_queue_push(dev, vq, elem, 0);
>          vu_queue_notify(dev, vq);
> -        g_free(elem);
> +        free(elem);
>      }
>  }
>
> diff --git a/contrib/vhost-user-gpu/virgl.c b/contrib/vhost-user-gpu/virgl.c
> index 43413e29df..b0bc22c3c1 100644
> --- a/contrib/vhost-user-gpu/virgl.c
> +++ b/contrib/vhost-user-gpu/virgl.c
> @@ -519,7 +519,7 @@ virgl_write_fence(void *opaque, uint32_t fence)
>          g_debug("FENCE %" PRIu64, cmd->cmd_hdr.fence_id);
>          vg_ctrl_response_nodata(g, cmd, VIRTIO_GPU_RESP_OK_NODATA);
>          QTAILQ_REMOVE(&g->fenceq, cmd, next);
> -        g_free(cmd);
> +        free(cmd);
>          g->inflight--;
>      }
>  }
> --
> 2.21.1
>



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH-for-5.0 2/3] virtio: Document virtqueue_pop()
  2020-03-23 11:29 ` [PATCH-for-5.0 2/3] virtio: Document virtqueue_pop() Philippe Mathieu-Daudé
@ 2020-03-23 11:54   ` Marc-André Lureau
  2020-03-25 17:50     ` Marc-André Lureau
  0 siblings, 1 reply; 13+ messages in thread
From: Marc-André Lureau @ 2020-03-23 11:54 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Peter Maydell, Michael S. Tsirkin, qemu-devel, Gerd Hoffmann

On Mon, Mar 23, 2020 at 12:30 PM Philippe Mathieu-Daudé
<philmd@redhat.com> wrote:
>
> Document that virtqueue_pop() returned memory must be released
> with free().
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>


> ---
>  include/hw/virtio/virtio.h | 8 ++++++++
>  1 file changed, 8 insertions(+)
>
> diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
> index b69d517496..c6e3bfc500 100644
> --- a/include/hw/virtio/virtio.h
> +++ b/include/hw/virtio/virtio.h
> @@ -199,6 +199,14 @@ void virtqueue_fill(VirtQueue *vq, const VirtQueueElement *elem,
>                      unsigned int len, unsigned int idx);
>
>  void virtqueue_map(VirtIODevice *vdev, VirtQueueElement *elem);
> +/**
> + * virtqueue_pop:
> + * @vq: a VirtQueue queue
> + * @sz: the size of struct to return (must be >= VirtQueueElement)
> + *
> + * Returns: a VirtQueueElement filled from the queue or NULL.
> + * The returned element must be free()-d by the caller.
> + */
>  void *virtqueue_pop(VirtQueue *vq, size_t sz);
>  unsigned int virtqueue_drop_all(VirtQueue *vq);
>  void *qemu_get_virtqueue_element(VirtIODevice *vdev, QEMUFile *f, size_t sz);
> --
> 2.21.1
>



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH-for-5.0 0/3] virtio, vhost-gpu: Release memory returned by malloc() with free()
  2020-03-23 11:29 [PATCH-for-5.0 0/3] virtio,vhost-gpu: Release memory returned by malloc() with free() Philippe Mathieu-Daudé
                   ` (2 preceding siblings ...)
  2020-03-23 11:29 ` [PATCH-for-5.0 3/3] virtio-gpu: Release memory returned by virtqueue_pop() with free() Philippe Mathieu-Daudé
@ 2020-03-23 13:41 ` Michael S. Tsirkin
  2020-04-13 10:59   ` Michael S. Tsirkin
  3 siblings, 1 reply; 13+ messages in thread
From: Michael S. Tsirkin @ 2020-03-23 13:41 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Peter Maydell, qemu-devel, Marc-André Lureau, Gerd Hoffmann

On Mon, Mar 23, 2020 at 12:29:40PM +0100, Philippe Mathieu-Daudé wrote:
> Coverity reported a ALLOC_FREE_MISMATCH in vg_handle_cursor(),
> because the memory returned by vu_queue_pop() is allocated with
> malloc(). Fix it.
> 
> Similar error occurs with virtio. Document and fix.

I will queue this. Thanks!

> Philippe Mathieu-Daudé (3):
>   vhost-user-gpu: Release memory returned by vu_queue_pop() with free()
>   virtio: Document virtqueue_pop()
>   virtio-gpu: Release memory returned by virtqueue_pop() with free()
> 
>  include/hw/virtio/virtio.h              | 8 ++++++++
>  contrib/vhost-user-gpu/vhost-user-gpu.c | 4 ++--
>  contrib/vhost-user-gpu/virgl.c          | 2 +-
>  hw/display/virtio-gpu-3d.c              | 2 +-
>  hw/display/virtio-gpu.c                 | 8 ++++----
>  5 files changed, 16 insertions(+), 8 deletions(-)
> 
> -- 
> 2.21.1



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH-for-5.0 3/3] virtio-gpu: Release memory returned by virtqueue_pop() with free()
  2020-03-23 11:29 ` [PATCH-for-5.0 3/3] virtio-gpu: Release memory returned by virtqueue_pop() with free() Philippe Mathieu-Daudé
@ 2020-03-25 14:31   ` Marc-André Lureau
  0 siblings, 0 replies; 13+ messages in thread
From: Marc-André Lureau @ 2020-03-25 14:31 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Peter Maydell, Michael S. Tsirkin, QEMU, Gerd Hoffmann

Hi

On Mon, Mar 23, 2020 at 12:31 PM Philippe Mathieu-Daudé
<philmd@redhat.com> wrote:
>
> As virtio_gpu_handle_ctrl() fills the cmdq calling virtqueue_pop(),
> we need to release it with free() in virtio_gpu_reset().
>
> virtio_gpu_handle_ctrl() allocates memory calling virtqueue_pop(),
> release it in virtio_gpu_process_cmdq() with free().
>
> virtio_gpu_process_cmdq() move commands from cmdq to fenceq, so
> we also have to release them with free().
>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>

nack, hw/virtio/virtio.c uses gmalloc (or I am missing something)

> ---
>  hw/display/virtio-gpu-3d.c | 2 +-
>  hw/display/virtio-gpu.c    | 8 ++++----
>  2 files changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/hw/display/virtio-gpu-3d.c b/hw/display/virtio-gpu-3d.c
> index 96621576c2..3a97d267e5 100644
> --- a/hw/display/virtio-gpu-3d.c
> +++ b/hw/display/virtio-gpu-3d.c
> @@ -506,7 +506,7 @@ static void virgl_write_fence(void *opaque, uint32_t fence)
>          trace_virtio_gpu_fence_resp(cmd->cmd_hdr.fence_id);
>          virtio_gpu_ctrl_response_nodata(g, cmd, VIRTIO_GPU_RESP_OK_NODATA);
>          QTAILQ_REMOVE(&g->fenceq, cmd, next);
> -        g_free(cmd);
> +        free(cmd);
>          g->inflight--;
>          if (virtio_gpu_stats_enabled(g->parent_obj.conf)) {
>              fprintf(stderr, "inflight: %3d (-)\r", g->inflight);
> diff --git a/hw/display/virtio-gpu.c b/hw/display/virtio-gpu.c
> index 5f0dd7c150..f5fbb722ee 100644
> --- a/hw/display/virtio-gpu.c
> +++ b/hw/display/virtio-gpu.c
> @@ -835,7 +835,7 @@ void virtio_gpu_process_cmdq(VirtIOGPU *g)
>                  fprintf(stderr, "inflight: %3d (+)\r", g->inflight);
>              }
>          } else {
> -            g_free(cmd);
> +            free(cmd);
>          }
>      }
>  }
> @@ -921,7 +921,7 @@ static void virtio_gpu_handle_cursor(VirtIODevice *vdev, VirtQueue *vq)
>          }
>          virtqueue_push(vq, elem, 0);
>          virtio_notify(vdev, vq);
> -        g_free(elem);
> +        free(elem);
>      }
>  }
>
> @@ -1157,14 +1157,14 @@ static void virtio_gpu_reset(VirtIODevice *vdev)
>      while (!QTAILQ_EMPTY(&g->cmdq)) {
>          cmd = QTAILQ_FIRST(&g->cmdq);
>          QTAILQ_REMOVE(&g->cmdq, cmd, next);
> -        g_free(cmd);
> +        free(cmd);
>      }
>
>      while (!QTAILQ_EMPTY(&g->fenceq)) {
>          cmd = QTAILQ_FIRST(&g->fenceq);
>          QTAILQ_REMOVE(&g->fenceq, cmd, next);
>          g->inflight--;
> -        g_free(cmd);
> +        free(cmd);
>      }
>
>  #ifdef CONFIG_VIRGL
> --
> 2.21.1
>
>


-- 
Marc-André Lureau


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH-for-5.0 2/3] virtio: Document virtqueue_pop()
  2020-03-23 11:54   ` Marc-André Lureau
@ 2020-03-25 17:50     ` Marc-André Lureau
  2020-03-25 20:51       ` Philippe Mathieu-Daudé
  0 siblings, 1 reply; 13+ messages in thread
From: Marc-André Lureau @ 2020-03-25 17:50 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Peter Maydell, Gerd Hoffmann, qemu-devel, Michael S. Tsirkin

On Mon, Mar 23, 2020 at 12:55 PM Marc-André Lureau
<marcandre.lureau@redhat.com> wrote:
>
> On Mon, Mar 23, 2020 at 12:30 PM Philippe Mathieu-Daudé
> <philmd@redhat.com> wrote:
> >
> > Document that virtqueue_pop() returned memory must be released
> > with free().
> >
> > Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>
> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

nack, hw/virtio/virtio.c uses g_malloc

>
>
> > ---
> >  include/hw/virtio/virtio.h | 8 ++++++++
> >  1 file changed, 8 insertions(+)
> >
> > diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
> > index b69d517496..c6e3bfc500 100644
> > --- a/include/hw/virtio/virtio.h
> > +++ b/include/hw/virtio/virtio.h
> > @@ -199,6 +199,14 @@ void virtqueue_fill(VirtQueue *vq, const VirtQueueElement *elem,
> >                      unsigned int len, unsigned int idx);
> >
> >  void virtqueue_map(VirtIODevice *vdev, VirtQueueElement *elem);
> > +/**
> > + * virtqueue_pop:
> > + * @vq: a VirtQueue queue
> > + * @sz: the size of struct to return (must be >= VirtQueueElement)
> > + *
> > + * Returns: a VirtQueueElement filled from the queue or NULL.
> > + * The returned element must be free()-d by the caller.
> > + */
> >  void *virtqueue_pop(VirtQueue *vq, size_t sz);
> >  unsigned int virtqueue_drop_all(VirtQueue *vq);
> >  void *qemu_get_virtqueue_element(VirtIODevice *vdev, QEMUFile *f, size_t sz);
> > --
> > 2.21.1
> >
>
>


-- 
Marc-André Lureau


^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH-for-5.0 2/3] virtio: Document virtqueue_pop()
  2020-03-25 17:50     ` Marc-André Lureau
@ 2020-03-25 20:51       ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 13+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-03-25 20:51 UTC (permalink / raw)
  To: Marc-André Lureau
  Cc: Peter Maydell, Gerd Hoffmann, qemu-devel, Michael S. Tsirkin

On 3/25/20 6:50 PM, Marc-André Lureau wrote:
> On Mon, Mar 23, 2020 at 12:55 PM Marc-André Lureau
> <marcandre.lureau@redhat.com> wrote:
>>
>> On Mon, Mar 23, 2020 at 12:30 PM Philippe Mathieu-Daudé
>> <philmd@redhat.com> wrote:
>>>
>>> Document that virtqueue_pop() returned memory must be released
>>> with free().
>>>
>>> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
>>
>> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> nack, hw/virtio/virtio.c uses g_malloc

Indeed... I opened hw/virtio/ and contrib/libvhost-user/ in the geany 
editor, and when I navigated from virtio.c to the 
virtqueue_alloc_element() definition I ended in libvhost-user.c:

static void *
virtqueue_alloc_element(size_t sz,
                                      unsigned out_num, unsigned in_num)
{
     VuVirtqElement *elem;
     size_t in_sg_ofs = ALIGN_UP(sz, __alignof__(elem->in_sg[0]));
     size_t out_sg_ofs = in_sg_ofs + in_num * sizeof(elem->in_sg[0]);
     size_t out_sg_end = out_sg_ofs + out_num * sizeof(elem->out_sg[0]);

     assert(sz >= sizeof(VuVirtqElement));
     elem = malloc(out_sg_end);
     elem->out_num = out_num;
     elem->in_num = in_num;
     elem->in_sg = (void *)elem + in_sg_ofs;
     elem->out_sg = (void *)elem + out_sg_ofs;
     return elem;
}

> 
>>
>>
>>> ---
>>>   include/hw/virtio/virtio.h | 8 ++++++++
>>>   1 file changed, 8 insertions(+)
>>>
>>> diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h
>>> index b69d517496..c6e3bfc500 100644
>>> --- a/include/hw/virtio/virtio.h
>>> +++ b/include/hw/virtio/virtio.h
>>> @@ -199,6 +199,14 @@ void virtqueue_fill(VirtQueue *vq, const VirtQueueElement *elem,
>>>                       unsigned int len, unsigned int idx);
>>>
>>>   void virtqueue_map(VirtIODevice *vdev, VirtQueueElement *elem);
>>> +/**
>>> + * virtqueue_pop:
>>> + * @vq: a VirtQueue queue
>>> + * @sz: the size of struct to return (must be >= VirtQueueElement)
>>> + *
>>> + * Returns: a VirtQueueElement filled from the queue or NULL.
>>> + * The returned element must be free()-d by the caller.
>>> + */
>>>   void *virtqueue_pop(VirtQueue *vq, size_t sz);
>>>   unsigned int virtqueue_drop_all(VirtQueue *vq);
>>>   void *qemu_get_virtqueue_element(VirtIODevice *vdev, QEMUFile *f, size_t sz);
>>> --
>>> 2.21.1
>>>
>>
>>
> 
> 



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH-for-5.0 0/3] virtio, vhost-gpu: Release memory returned by malloc() with free()
  2020-03-23 13:41 ` [PATCH-for-5.0 0/3] virtio, vhost-gpu: Release memory returned by malloc() " Michael S. Tsirkin
@ 2020-04-13 10:59   ` Michael S. Tsirkin
  2020-04-14 12:40     ` Philippe Mathieu-Daudé
  0 siblings, 1 reply; 13+ messages in thread
From: Michael S. Tsirkin @ 2020-04-13 10:59 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Peter Maydell, qemu-devel, Marc-André Lureau, Gerd Hoffmann

On Mon, Mar 23, 2020 at 09:41:20AM -0400, Michael S. Tsirkin wrote:
> On Mon, Mar 23, 2020 at 12:29:40PM +0100, Philippe Mathieu-Daudé wrote:
> > Coverity reported a ALLOC_FREE_MISMATCH in vg_handle_cursor(),
> > because the memory returned by vu_queue_pop() is allocated with
> > malloc(). Fix it.
> > 
> > Similar error occurs with virtio. Document and fix.
> 
> I will queue this. Thanks!

So what are we doing with this patchset? Marc-André reported issues -
any plan to fix them up? Split up the patchset to 3 independent
patches?

> > Philippe Mathieu-Daudé (3):
> >   vhost-user-gpu: Release memory returned by vu_queue_pop() with free()
> >   virtio: Document virtqueue_pop()
> >   virtio-gpu: Release memory returned by virtqueue_pop() with free()
> > 
> >  include/hw/virtio/virtio.h              | 8 ++++++++
> >  contrib/vhost-user-gpu/vhost-user-gpu.c | 4 ++--
> >  contrib/vhost-user-gpu/virgl.c          | 2 +-
> >  hw/display/virtio-gpu-3d.c              | 2 +-
> >  hw/display/virtio-gpu.c                 | 8 ++++----
> >  5 files changed, 16 insertions(+), 8 deletions(-)
> > 
> > -- 
> > 2.21.1



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH-for-5.0 0/3] virtio, vhost-gpu: Release memory returned by malloc() with free()
  2020-04-13 10:59   ` Michael S. Tsirkin
@ 2020-04-14 12:40     ` Philippe Mathieu-Daudé
  0 siblings, 0 replies; 13+ messages in thread
From: Philippe Mathieu-Daudé @ 2020-04-14 12:40 UTC (permalink / raw)
  To: Michael S. Tsirkin
  Cc: Peter Maydell, qemu-devel, Marc-André Lureau, Gerd Hoffmann

On 4/13/20 12:59 PM, Michael S. Tsirkin wrote:
> On Mon, Mar 23, 2020 at 09:41:20AM -0400, Michael S. Tsirkin wrote:
>> On Mon, Mar 23, 2020 at 12:29:40PM +0100, Philippe Mathieu-Daudé wrote:
>>> Coverity reported a ALLOC_FREE_MISMATCH in vg_handle_cursor(),
>>> because the memory returned by vu_queue_pop() is allocated with
>>> malloc(). Fix it.
>>>
>>> Similar error occurs with virtio. Document and fix.
>>
>> I will queue this. Thanks!
> 
> So what are we doing with this patchset? Marc-André reported issues -
> any plan to fix them up? Split up the patchset to 3 independent
> patches?

As noted Marc-André, patches 2 & 3 are incorrect.

Patch 1 can be queued as it with no modification.

> 
>>> Philippe Mathieu-Daudé (3):
>>>    vhost-user-gpu: Release memory returned by vu_queue_pop() with free()
>>>    virtio: Document virtqueue_pop()
>>>    virtio-gpu: Release memory returned by virtqueue_pop() with free()
>>>
>>>   include/hw/virtio/virtio.h              | 8 ++++++++
>>>   contrib/vhost-user-gpu/vhost-user-gpu.c | 4 ++--
>>>   contrib/vhost-user-gpu/virgl.c          | 2 +-
>>>   hw/display/virtio-gpu-3d.c              | 2 +-
>>>   hw/display/virtio-gpu.c                 | 8 ++++----
>>>   5 files changed, 16 insertions(+), 8 deletions(-)
>>>
>>> -- 
>>> 2.21.1
> 



^ permalink raw reply	[flat|nested] 13+ messages in thread

* Re: [PATCH-for-5.0 1/3] vhost-user-gpu: Release memory returned by vu_queue_pop() with free()
  2020-03-23 11:29 ` [PATCH-for-5.0 1/3] vhost-user-gpu: Release memory returned by vu_queue_pop() " Philippe Mathieu-Daudé
  2020-03-23 11:54   ` Marc-André Lureau
@ 2020-04-15 19:27   ` Peter Maydell
  1 sibling, 0 replies; 13+ messages in thread
From: Peter Maydell @ 2020-04-15 19:27 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé
  Cc: Marc-André Lureau, qemu-stable, Michael S. Tsirkin,
	QEMU Developers, Gerd Hoffmann

On Mon, 23 Mar 2020 at 11:29, Philippe Mathieu-Daudé <philmd@redhat.com> wrote:
>
> vu_queue_pop() returns memory that must be freed with free().
>
> Cc: qemu-stable@nongnu.org
> Reported-by: Coverity (CID 1421887 ALLOC_FREE_MISMATCH)
> Suggested-by: Peter Maydell <peter.maydell@linaro.org>
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  contrib/vhost-user-gpu/vhost-user-gpu.c | 4 ++--
>  contrib/vhost-user-gpu/virgl.c          | 2 +-
>  2 files changed, 3 insertions(+), 3 deletions(-)

Applied this patch (and not any of the rest of the series)
to master for rc3, thanks.

-- PMM


^ permalink raw reply	[flat|nested] 13+ messages in thread

end of thread, other threads:[~2020-04-15 19:28 UTC | newest]

Thread overview: 13+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-23 11:29 [PATCH-for-5.0 0/3] virtio,vhost-gpu: Release memory returned by malloc() with free() Philippe Mathieu-Daudé
2020-03-23 11:29 ` [PATCH-for-5.0 1/3] vhost-user-gpu: Release memory returned by vu_queue_pop() " Philippe Mathieu-Daudé
2020-03-23 11:54   ` Marc-André Lureau
2020-04-15 19:27   ` Peter Maydell
2020-03-23 11:29 ` [PATCH-for-5.0 2/3] virtio: Document virtqueue_pop() Philippe Mathieu-Daudé
2020-03-23 11:54   ` Marc-André Lureau
2020-03-25 17:50     ` Marc-André Lureau
2020-03-25 20:51       ` Philippe Mathieu-Daudé
2020-03-23 11:29 ` [PATCH-for-5.0 3/3] virtio-gpu: Release memory returned by virtqueue_pop() with free() Philippe Mathieu-Daudé
2020-03-25 14:31   ` Marc-André Lureau
2020-03-23 13:41 ` [PATCH-for-5.0 0/3] virtio, vhost-gpu: Release memory returned by malloc() " Michael S. Tsirkin
2020-04-13 10:59   ` Michael S. Tsirkin
2020-04-14 12:40     ` Philippe Mathieu-Daudé

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).