| Message ID | 1575856810-9388-2-git-send-email-pannengyuan@huawei.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | virtio: fix memory leak in virtio-balloon/virtio-serial-bus | expand |
On 2019/12/9 10:00, pannengyuan@huawei.com wrote: > From: Michael S. Tsirkin <mst@redhat.com> > > Devices tend to maintain vq pointers, allow deleting them through a vq > pointer. > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com> > [PMM: change function name to virtio_queue_cleanup; set used_elems to NULL after free] Oh. I'm sorry. Here is PNM (not PMM). > Cc: Amit Shah <amit@kernel.org> > Reviewed-by: Pankaj Gupta <pagupta@redhat.com> > Reviewed-by: Laurent Vivier <lvivier@redhat.com> > --- > Changes v2 to v1: > - use virtio_delete_queue to cleanup vq through a vq pointer > --- > Changes v3 to v2: > - change function name from virtio_delete_queue to virtio_queue_cleanup > --- > hw/virtio/virtio.c | 16 +++++++++++----- > include/hw/virtio/virtio.h | 2 ++ > 2 files changed, 13 insertions(+), 5 deletions(-) > > diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c > index 04716b5..2743258 100644 > --- a/hw/virtio/virtio.c > +++ b/hw/virtio/virtio.c > @@ -2330,17 +2330,23 @@ VirtQueue *virtio_add_queue(VirtIODevice *vdev, int queue_size, > return &vdev->vq[i]; > } > > +void virtio_queue_cleanup(VirtQueue *vq) > +{ > + vq->vring.num = 0; > + vq->vring.num_default = 0; > + vq->handle_output = NULL; > + vq->handle_aio_output = NULL; > + g_free(vq->used_elems); > + vq->used_elems = NULL; > +} > + > void virtio_del_queue(VirtIODevice *vdev, int n) > { > if (n < 0 || n >= VIRTIO_QUEUE_MAX) { > abort(); > } > > - vdev->vq[n].vring.num = 0; > - vdev->vq[n].vring.num_default = 0; > - vdev->vq[n].handle_output = NULL; > - vdev->vq[n].handle_aio_output = NULL; > - g_free(vdev->vq[n].used_elems); > + virtio_queue_cleanup(&vdev->vq[n]); > } > > static void virtio_set_isr(VirtIODevice *vdev, int value) > diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h > index c32a815..cc0b3f0 100644 > --- a/include/hw/virtio/virtio.h > +++ b/include/hw/virtio/virtio.h > @@ -183,6 +183,8 @@ VirtQueue *virtio_add_queue(VirtIODevice *vdev, int queue_size, > > void virtio_del_queue(VirtIODevice *vdev, int n); > > +void virtio_queue_cleanup(VirtQueue *vq); > + > void virtqueue_push(VirtQueue *vq, const VirtQueueElement *elem, > unsigned int len); > void virtqueue_flush(VirtQueue *vq, unsigned int count); >
On 09.12.19 03:00, pannengyuan@huawei.com wrote: > From: Michael S. Tsirkin <mst@redhat.com> > > Devices tend to maintain vq pointers, allow deleting them through a vq > pointer. > > Signed-off-by: Michael S. Tsirkin <mst@redhat.com> > Signed-off-by: Pan Nengyuan <pannengyuan@huawei.com> > [PMM: change function name to virtio_queue_cleanup; set used_elems to NULL after free] > Cc: Amit Shah <amit@kernel.org> > Reviewed-by: Pankaj Gupta <pagupta@redhat.com> > Reviewed-by: Laurent Vivier <lvivier@redhat.com> > --- > Changes v2 to v1: > - use virtio_delete_queue to cleanup vq through a vq pointer > --- > Changes v3 to v2: > - change function name from virtio_delete_queue to virtio_queue_cleanup > --- > hw/virtio/virtio.c | 16 +++++++++++----- > include/hw/virtio/virtio.h | 2 ++ > 2 files changed, 13 insertions(+), 5 deletions(-) > > diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c > index 04716b5..2743258 100644 > --- a/hw/virtio/virtio.c > +++ b/hw/virtio/virtio.c > @@ -2330,17 +2330,23 @@ VirtQueue *virtio_add_queue(VirtIODevice *vdev, int queue_size, > return &vdev->vq[i]; > } > > +void virtio_queue_cleanup(VirtQueue *vq) > +{ > + vq->vring.num = 0; > + vq->vring.num_default = 0; > + vq->handle_output = NULL; > + vq->handle_aio_output = NULL; > + g_free(vq->used_elems); > + vq->used_elems = NULL; > +} > + > void virtio_del_queue(VirtIODevice *vdev, int n) > { > if (n < 0 || n >= VIRTIO_QUEUE_MAX) { > abort(); > } > > - vdev->vq[n].vring.num = 0; > - vdev->vq[n].vring.num_default = 0; > - vdev->vq[n].handle_output = NULL; > - vdev->vq[n].handle_aio_output = NULL; > - g_free(vdev->vq[n].used_elems); > + virtio_queue_cleanup(&vdev->vq[n]); > } > > static void virtio_set_isr(VirtIODevice *vdev, int value) > diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h > index c32a815..cc0b3f0 100644 > --- a/include/hw/virtio/virtio.h > +++ b/include/hw/virtio/virtio.h > @@ -183,6 +183,8 @@ VirtQueue *virtio_add_queue(VirtIODevice *vdev, int queue_size, > > void virtio_del_queue(VirtIODevice *vdev, int n); > > +void virtio_queue_cleanup(VirtQueue *vq); > + > void virtqueue_push(VirtQueue *vq, const VirtQueueElement *elem, > unsigned int len); > void virtqueue_flush(VirtQueue *vq, unsigned int count); > Reviewed-by: David Hildenbrand <david@redhat.com>
diff --git a/hw/virtio/virtio.c b/hw/virtio/virtio.c index 04716b5..2743258 100644 --- a/hw/virtio/virtio.c +++ b/hw/virtio/virtio.c @@ -2330,17 +2330,23 @@ VirtQueue *virtio_add_queue(VirtIODevice *vdev, int queue_size, return &vdev->vq[i]; } +void virtio_queue_cleanup(VirtQueue *vq) +{ + vq->vring.num = 0; + vq->vring.num_default = 0; + vq->handle_output = NULL; + vq->handle_aio_output = NULL; + g_free(vq->used_elems); + vq->used_elems = NULL; +} + void virtio_del_queue(VirtIODevice *vdev, int n) { if (n < 0 || n >= VIRTIO_QUEUE_MAX) { abort(); } - vdev->vq[n].vring.num = 0; - vdev->vq[n].vring.num_default = 0; - vdev->vq[n].handle_output = NULL; - vdev->vq[n].handle_aio_output = NULL; - g_free(vdev->vq[n].used_elems); + virtio_queue_cleanup(&vdev->vq[n]); } static void virtio_set_isr(VirtIODevice *vdev, int value) diff --git a/include/hw/virtio/virtio.h b/include/hw/virtio/virtio.h index c32a815..cc0b3f0 100644 --- a/include/hw/virtio/virtio.h +++ b/include/hw/virtio/virtio.h @@ -183,6 +183,8 @@ VirtQueue *virtio_add_queue(VirtIODevice *vdev, int queue_size, void virtio_del_queue(VirtIODevice *vdev, int n); +void virtio_queue_cleanup(VirtQueue *vq); + void virtqueue_push(VirtQueue *vq, const VirtQueueElement *elem, unsigned int len); void virtqueue_flush(VirtQueue *vq, unsigned int count);