Re: [Qemu-devel] [PATCH-for-4.1 v2 2/3] virtio-balloon: fix memory leak on unrealize()

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 2966 bytes --]

On Wed, Jul 17, 2019 at 12:35:49PM +0200, David Hildenbrand wrote:
> We could have tracking data for a pbp (PartiallyBalloonedPage)
> allocated. Let's free it.
> 
> Fixes: ed48c59875b6 ("virtio-balloon: Safely handle BALLOON_PAGE_SIZE <
>                      host page size")
> Cc: qemu-stable@nongnu.org #v4.0.0
> Cc: Stefan Hajnoczi <stefanha@redhat.com>
> Cc: David Gibson <david@gibson.dropbear.id.au>
> Cc: Michael S. Tsirkin <mst@redhat.com>
> Cc: Igor Mammedov <imammedo@redhat.com>
> Signed-off-by: David Hildenbrand <david@redhat.com>

Acked-by: David Gibson <david@gibson.dropbear.id.au>

> ---
>  hw/virtio/virtio-balloon.c | 16 ++++++++++------
>  1 file changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
> index 669067d661..84d01bceb3 100644
> --- a/hw/virtio/virtio-balloon.c
> +++ b/hw/virtio/virtio-balloon.c
> @@ -40,6 +40,12 @@ struct PartiallyBalloonedPage {
>      unsigned long bitmap[];
>  };
>  
> +static void virtio_balloon_reset_pbp(VirtIOBalloon *balloon)
> +{
> +    g_free(balloon->pbp);
> +    balloon->pbp = NULL;
> +}
> +
>  static void balloon_inflate_page(VirtIOBalloon *balloon,
>                                   MemoryRegion *mr, hwaddr offset)
>  {
> @@ -82,8 +88,7 @@ static void balloon_inflate_page(VirtIOBalloon *balloon,
>          /* We've partially ballooned part of a host page, but now
>           * we're trying to balloon part of a different one.  Too hard,
>           * give up on the old partial page */
> -        g_free(balloon->pbp);
> -        balloon->pbp = NULL;
> +        virtio_balloon_reset_pbp(balloon);
>      }
>  
>      if (!balloon->pbp) {
> @@ -106,8 +111,7 @@ static void balloon_inflate_page(VirtIOBalloon *balloon,
>           * has already reported them, and failing to discard a balloon
>           * page is not fatal */
>  
> -        g_free(balloon->pbp);
> -        balloon->pbp = NULL;
> +        virtio_balloon_reset_pbp(balloon);
>      }
>  }
>  
> @@ -143,8 +147,7 @@ static void balloon_deflate_page(VirtIOBalloon *balloon,
>                    balloon->pbp->bitmap);
>  
>          if (bitmap_empty(balloon->pbp->bitmap, subpages)) {
> -            g_free(balloon->pbp);
> -            balloon->pbp = NULL;
> +            virtio_balloon_reset_pbp(balloon);
>          }
>      }
>  
> @@ -831,6 +834,7 @@ static void virtio_balloon_device_unrealize(DeviceState *dev, Error **errp)
>          virtio_balloon_free_page_stop(s);
>          precopy_remove_notifier(&s->free_page_report_notify);
>      }
> +    virtio_balloon_reset_pbp(s);
>      balloon_stats_destroy_timer(s);
>      qemu_remove_balloon_handler(s);
>      virtio_cleanup(vdev);

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]