hw/virtio/virtio-balloon.c | 18 ++++++++---------- 1 file changed, 8 insertions(+), 10 deletions(-)
Previous patches switched to a temporary pbp but that does not go far
enough: after device uses a buffer, guest is free to reuse it, so
tracking the page and freeing it later is wrong.
Free and reset the pbp after we push each element.
Fixes: ed48c59875b6 ("virtio-balloon: Safely handle BALLOON_PAGE_SIZE < host page size")
Cc: qemu-stable@nongnu.org #v4.0.0
Cc: David Hildenbrand <david@redhat.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
hw/virtio/virtio-balloon.c | 18 ++++++++----------
1 file changed, 8 insertions(+), 10 deletions(-)
diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
index fe9664e42c..460a702463 100644
--- a/hw/virtio/virtio-balloon.c
+++ b/hw/virtio/virtio-balloon.c
@@ -39,13 +39,14 @@ typedef struct PartiallyBalloonedPage {
unsigned long *bitmap;
} PartiallyBalloonedPage;
-static void virtio_balloon_pbp_free(PartiallyBalloonedPage *pbp)
+static void virtio_balloon_pbp_free(PartiallyBalloonedPage **pbp)
{
- if (!pbp) {
+ if (!*pbp) {
return;
}
- g_free(pbp->bitmap);
- g_free(pbp);
+ g_free(*pbp->bitmap);
+ g_free(*pbp);
+ *pbp = NULL;
}
static PartiallyBalloonedPage *virtio_balloon_pbp_alloc(ram_addr_t base_gpa,
@@ -108,8 +109,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 */
- virtio_balloon_pbp_free(*pbp);
- *pbp = NULL;
+ virtio_balloon_pbp_free(pbp);
}
if (!*pbp) {
@@ -127,8 +127,7 @@ static void balloon_inflate_page(VirtIOBalloon *balloon,
/* We ignore errors from ram_block_discard_range(), because it
* has already reported them, and failing to discard a balloon
* page is not fatal */
- virtio_balloon_pbp_free(*pbp);
- *pbp = NULL;
+ virtio_balloon_pbp_free(pbp);
}
}
@@ -379,9 +378,8 @@ static void virtio_balloon_handle_output(VirtIODevice *vdev, VirtQueue *vq)
virtqueue_push(vq, elem, offset);
virtio_notify(vdev, vq);
g_free(elem);
+ virtio_balloon_pbp_free(&pbp);
}
-
- virtio_balloon_pbp_free(pbp);
}
static void virtio_balloon_receive_stats(VirtIODevice *vdev, VirtQueue *vq)
--
MST
> -static void virtio_balloon_pbp_free(PartiallyBalloonedPage *pbp) > +static void virtio_balloon_pbp_free(PartiallyBalloonedPage **pbp) > { > - if (!pbp) { > + if (!*pbp) { > return; > } > - g_free(pbp->bitmap); > - g_free(pbp); > + g_free(*pbp->bitmap); This has to be g_free((*pbp)->bitmap); to compile. > + g_free(*pbp); > + *pbp = NULL; > } > > static PartiallyBalloonedPage *virtio_balloon_pbp_alloc(ram_addr_t base_gpa, > @@ -108,8 +109,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 */ > - virtio_balloon_pbp_free(*pbp); > - *pbp = NULL; > + virtio_balloon_pbp_free(pbp); > } > > if (!*pbp) { > @@ -127,8 +127,7 @@ static void balloon_inflate_page(VirtIOBalloon *balloon, > /* We ignore errors from ram_block_discard_range(), because it > * has already reported them, and failing to discard a balloon > * page is not fatal */ > - virtio_balloon_pbp_free(*pbp); > - *pbp = NULL; > + virtio_balloon_pbp_free(pbp); > } > } > > @@ -379,9 +378,8 @@ static void virtio_balloon_handle_output(VirtIODevice *vdev, VirtQueue *vq) > virtqueue_push(vq, elem, offset); > virtio_notify(vdev, vq); > g_free(elem); > + virtio_balloon_pbp_free(&pbp); You could move the "PartiallyBalloonedPage *pbp = NULL;" now into the loop. > } > - > - virtio_balloon_pbp_free(pbp); > } > > static void virtio_balloon_receive_stats(VirtIODevice *vdev, VirtQueue *vq) > Gave it a quick test with a hugepage backing on x86 - still works. Would be good if somebody could test with 64k PPC guest. Reviewed-by: David Hildenbrand <david@redhat.com> -- Thanks, David / dhildenb
© 2016 - 2024 Red Hat, Inc.