From: David Hildenbrand <david@redhat.com> To: qemu-devel@nongnu.org Cc: "Michael S . Tsirkin" <mst@redhat.com>, David Hildenbrand <david@redhat.com>, qemu-stable@nongnu.org, Stefan Hajnoczi <stefanha@redhat.com>, Igor Mammedov <imammedo@redhat.com>, David Gibson <david@gibson.dropbear.id.au> Subject: [Qemu-devel] [PATCH-for-4.1 v3 5/6] virtio-balloon: Rework pbp tracking data Date: Mon, 22 Jul 2019 15:41:07 +0200 [thread overview] Message-ID: <20190722134108.22151-6-david@redhat.com> (raw) In-Reply-To: <20190722134108.22151-1-david@redhat.com> Using the address of a RAMBlock to test for a matching pbp is not really safe. Instead, let's use the guest physical address of the base page along with the page size (via the number of subpages). Also, let's allocate the bitmap separately. This makes the code easier to read and maintain - we can reuse bitmap_new(). Prepare the code to move the PBP out of the device. Fixes: ed48c59875b6 ("virtio-balloon: Safely handle BALLOON_PAGE_SIZE < host page size") Fixes: b27b32391404 ("virtio-balloon: Fix possible guest memory corruption with inflates & deflates") Cc: qemu-stable@nongnu.org #v4.0.0 Signed-off-by: David Hildenbrand <david@redhat.com> --- hw/virtio/virtio-balloon.c | 69 +++++++++++++++++++++++++------------- 1 file changed, 46 insertions(+), 23 deletions(-) diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c index f206cc8bf7..40d493a31a 100644 --- a/hw/virtio/virtio-balloon.c +++ b/hw/virtio/virtio-balloon.c @@ -35,16 +35,44 @@ #define BALLOON_PAGE_SIZE (1 << VIRTIO_BALLOON_PFN_SHIFT) struct PartiallyBalloonedPage { - RAMBlock *rb; - ram_addr_t base; - unsigned long bitmap[]; + ram_addr_t base_gpa; + long subpages; + unsigned long *bitmap; }; +static void virtio_balloon_pbp_free(PartiallyBalloonedPage *pbp) +{ + if (!pbp) { + return; + } + g_free(pbp->bitmap); + g_free(pbp); +} + +static PartiallyBalloonedPage *virtio_balloon_pbp_alloc(ram_addr_t base_gpa, + long subpages) +{ + PartiallyBalloonedPage *pbp = g_new0(PartiallyBalloonedPage, 1); + + pbp->base_gpa = base_gpa; + pbp->subpages = subpages; + pbp->bitmap = bitmap_new(subpages); + + return pbp; +} + +static bool virtio_balloon_pbp_matches(PartiallyBalloonedPage *pbp, + ram_addr_t base_gpa, long subpages) +{ + return pbp->subpages == subpages && pbp->base_gpa == base_gpa; +} + static void balloon_inflate_page(VirtIOBalloon *balloon, MemoryRegion *mr, hwaddr mr_offset) { void *addr = memory_region_get_ram_ptr(mr) + mr_offset; - ram_addr_t rb_offset, rb_aligned_offset; + ram_addr_t rb_offset, rb_aligned_offset, base_gpa; + PartiallyBalloonedPage **pbp = &balloon->pbp; RAMBlock *rb; size_t rb_page_size; int subpages; @@ -75,39 +103,34 @@ static void balloon_inflate_page(VirtIOBalloon *balloon, rb_aligned_offset = QEMU_ALIGN_DOWN(rb_offset, rb_page_size); subpages = rb_page_size / BALLOON_PAGE_SIZE; + base_gpa = memory_region_get_ram_addr(mr) + mr_offset - + (rb_offset - rb_aligned_offset); - if (balloon->pbp - && (rb != balloon->pbp->rb - || rb_aligned_offset != balloon->pbp->base)) { + if (*pbp && !virtio_balloon_pbp_matches(*pbp, base_gpa, subpages)) { /* 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_pbp_free(*pbp); + *pbp = NULL; } - if (!balloon->pbp) { - /* Starting on a new host page */ - size_t bitlen = BITS_TO_LONGS(subpages) * sizeof(unsigned long); - balloon->pbp = g_malloc0(sizeof(PartiallyBalloonedPage) + bitlen); - balloon->pbp->rb = rb; - balloon->pbp->base = rb_aligned_offset; + if (!*pbp) { + *pbp = virtio_balloon_pbp_alloc(base_gpa, subpages); } - set_bit((rb_offset - balloon->pbp->base) / BALLOON_PAGE_SIZE, - balloon->pbp->bitmap); + set_bit((rb_offset - rb_aligned_offset) / BALLOON_PAGE_SIZE, + (*pbp)->bitmap); - if (bitmap_full(balloon->pbp->bitmap, subpages)) { + if (bitmap_full((*pbp)->bitmap, subpages)) { /* We've accumulated a full host page, we can actually discard * it now */ - ram_block_discard_range(rb, balloon->pbp->base, rb_page_size); + ram_block_discard_range(rb, rb_aligned_offset, rb_page_size); /* We ignore errors from ram_block_discard_range(), because it * has already reported them, and failing to discard a balloon * page is not fatal */ - - g_free(balloon->pbp); - balloon->pbp = NULL; + virtio_balloon_pbp_free(*pbp); + *pbp = NULL; } } @@ -128,7 +151,7 @@ static void balloon_deflate_page(VirtIOBalloon *balloon, if (balloon->pbp) { /* Let's play safe and always reset the pbp on deflation requests. */ - g_free(balloon->pbp); + virtio_balloon_pbp_free(balloon->pbp); balloon->pbp = NULL; } -- 2.21.0
WARNING: multiple messages have this Message-ID (diff)
From: "Michael S. Tsirkin" <mst@redhat.com> To: qemu-devel@nongnu.org Cc: Peter Maydell <peter.maydell@linaro.org>, qemu-stable@nongnu.org, David Hildenbrand <david@redhat.com> Subject: [Qemu-devel] [PULL 09/12] virtio-balloon: Rework pbp tracking data Date: Thu, 25 Jul 2019 11:32:09 -0400 [thread overview] Message-ID: <20190722134108.22151-6-david@redhat.com> (raw) Message-ID: <20190725153209.ZFTBLuIg-3SGwSf1hqRq0-DcSYtzL4Li5ikikrRrxAI@z> (raw) In-Reply-To: <20190725153059.7313-1-mst@redhat.com> From: David Hildenbrand <david@redhat.com> Using the address of a RAMBlock to test for a matching pbp is not really safe. Instead, let's use the guest physical address of the base page along with the page size (via the number of subpages). Also, let's allocate the bitmap separately. This makes the code easier to read and maintain - we can reuse bitmap_new(). Prepare the code to move the PBP out of the device. Fixes: ed48c59875b6 ("virtio-balloon: Safely handle BALLOON_PAGE_SIZE < host page size") Fixes: b27b32391404 ("virtio-balloon: Fix possible guest memory corruption with inflates & deflates") Cc: qemu-stable@nongnu.org #v4.0.0 Signed-off-by: David Hildenbrand <david@redhat.com> Message-Id: <20190722134108.22151-6-david@redhat.com> Reviewed-by: Michael S. Tsirkin <mst@redhat.com> Signed-off-by: Michael S. Tsirkin <mst@redhat.com> --- hw/virtio/virtio-balloon.c | 69 +++++++++++++++++++++++++------------- 1 file changed, 46 insertions(+), 23 deletions(-) diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c index f206cc8bf7..40d493a31a 100644 --- a/hw/virtio/virtio-balloon.c +++ b/hw/virtio/virtio-balloon.c @@ -35,16 +35,44 @@ #define BALLOON_PAGE_SIZE (1 << VIRTIO_BALLOON_PFN_SHIFT) struct PartiallyBalloonedPage { - RAMBlock *rb; - ram_addr_t base; - unsigned long bitmap[]; + ram_addr_t base_gpa; + long subpages; + unsigned long *bitmap; }; +static void virtio_balloon_pbp_free(PartiallyBalloonedPage *pbp) +{ + if (!pbp) { + return; + } + g_free(pbp->bitmap); + g_free(pbp); +} + +static PartiallyBalloonedPage *virtio_balloon_pbp_alloc(ram_addr_t base_gpa, + long subpages) +{ + PartiallyBalloonedPage *pbp = g_new0(PartiallyBalloonedPage, 1); + + pbp->base_gpa = base_gpa; + pbp->subpages = subpages; + pbp->bitmap = bitmap_new(subpages); + + return pbp; +} + +static bool virtio_balloon_pbp_matches(PartiallyBalloonedPage *pbp, + ram_addr_t base_gpa, long subpages) +{ + return pbp->subpages == subpages && pbp->base_gpa == base_gpa; +} + static void balloon_inflate_page(VirtIOBalloon *balloon, MemoryRegion *mr, hwaddr mr_offset) { void *addr = memory_region_get_ram_ptr(mr) + mr_offset; - ram_addr_t rb_offset, rb_aligned_offset; + ram_addr_t rb_offset, rb_aligned_offset, base_gpa; + PartiallyBalloonedPage **pbp = &balloon->pbp; RAMBlock *rb; size_t rb_page_size; int subpages; @@ -75,39 +103,34 @@ static void balloon_inflate_page(VirtIOBalloon *balloon, rb_aligned_offset = QEMU_ALIGN_DOWN(rb_offset, rb_page_size); subpages = rb_page_size / BALLOON_PAGE_SIZE; + base_gpa = memory_region_get_ram_addr(mr) + mr_offset - + (rb_offset - rb_aligned_offset); - if (balloon->pbp - && (rb != balloon->pbp->rb - || rb_aligned_offset != balloon->pbp->base)) { + if (*pbp && !virtio_balloon_pbp_matches(*pbp, base_gpa, subpages)) { /* 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_pbp_free(*pbp); + *pbp = NULL; } - if (!balloon->pbp) { - /* Starting on a new host page */ - size_t bitlen = BITS_TO_LONGS(subpages) * sizeof(unsigned long); - balloon->pbp = g_malloc0(sizeof(PartiallyBalloonedPage) + bitlen); - balloon->pbp->rb = rb; - balloon->pbp->base = rb_aligned_offset; + if (!*pbp) { + *pbp = virtio_balloon_pbp_alloc(base_gpa, subpages); } - set_bit((rb_offset - balloon->pbp->base) / BALLOON_PAGE_SIZE, - balloon->pbp->bitmap); + set_bit((rb_offset - rb_aligned_offset) / BALLOON_PAGE_SIZE, + (*pbp)->bitmap); - if (bitmap_full(balloon->pbp->bitmap, subpages)) { + if (bitmap_full((*pbp)->bitmap, subpages)) { /* We've accumulated a full host page, we can actually discard * it now */ - ram_block_discard_range(rb, balloon->pbp->base, rb_page_size); + ram_block_discard_range(rb, rb_aligned_offset, rb_page_size); /* We ignore errors from ram_block_discard_range(), because it * has already reported them, and failing to discard a balloon * page is not fatal */ - - g_free(balloon->pbp); - balloon->pbp = NULL; + virtio_balloon_pbp_free(*pbp); + *pbp = NULL; } } @@ -128,7 +151,7 @@ static void balloon_deflate_page(VirtIOBalloon *balloon, if (balloon->pbp) { /* Let's play safe and always reset the pbp on deflation requests. */ - g_free(balloon->pbp); + virtio_balloon_pbp_free(balloon->pbp); balloon->pbp = NULL; } -- MST
next prev parent reply other threads:[~2019-07-22 13:42 UTC|newest] Thread overview: 53+ messages / expand[flat|nested] mbox.gz Atom feed top 2019-07-22 13:41 [Qemu-devel] [PATCH-for-4.1 v3 0/6] virtio-balloon: fixes David Hildenbrand 2019-07-22 13:41 ` [Qemu-devel] [PATCH-for-4.1 v3 1/6] virtio-balloon: Fix wrong sign extension of PFNs David Hildenbrand 2019-07-23 2:27 ` David Gibson 2019-07-25 15:31 ` [Qemu-devel] [PULL 05/12] " Michael S. Tsirkin 2019-07-22 13:41 ` [Qemu-devel] [PATCH-for-4.1 v3 2/6] virtio-balloon: Fix QEMU crashes on pagesize > BALLOON_PAGE_SIZE David Hildenbrand 2019-07-25 15:31 ` [Qemu-devel] [PULL 06/12] " Michael S. Tsirkin 2019-07-22 13:41 ` [Qemu-devel] [PATCH-for-4.1 v3 3/6] virtio-balloon: Simplify deflate with pbp David Hildenbrand 2019-07-25 15:32 ` [Qemu-devel] [PULL 07/12] " Michael S. Tsirkin 2019-07-22 13:41 ` [Qemu-devel] [PATCH-for-4.1 v3 4/6] virtio-balloon: Better names for offset variables in inflate/deflate code David Hildenbrand 2019-07-25 15:32 ` [Qemu-devel] [PULL 08/12] " Michael S. Tsirkin 2019-07-22 13:41 ` David Hildenbrand [this message] 2019-07-23 2:54 ` [Qemu-devel] [PATCH-for-4.1 v3 5/6] virtio-balloon: Rework pbp tracking data David Gibson 2019-07-23 7:38 ` David Hildenbrand 2019-07-25 15:32 ` [Qemu-devel] [PULL 09/12] " Michael S. Tsirkin 2019-07-22 13:41 ` [Qemu-devel] [PATCH-for-4.1 v3 6/6] virtio-balloon: Use temporary PBP only David Hildenbrand 2019-07-23 3:22 ` David Gibson 2019-07-25 15:32 ` [Qemu-devel] [PULL 10/12] " Michael S. Tsirkin 2019-07-25 11:36 [Qemu-devel] [PATCH-for-4.1 v4 0/7] virtio-balloon: fixes David Hildenbrand 2019-07-25 11:36 ` [Qemu-devel] [PATCH-for-4.1 v4 1/7] virtio-balloon: Fix wrong sign extension of PFNs David Hildenbrand 2019-07-25 12:36 ` Pankaj Gupta 2019-07-25 11:36 ` [Qemu-devel] [PATCH-for-4.1 v4 2/7] virtio-balloon: Fix QEMU crashes on pagesize > BALLOON_PAGE_SIZE David Hildenbrand 2019-07-25 11:36 ` [Qemu-devel] [PATCH-for-4.1 v4 3/7] virtio-balloon: Simplify deflate with pbp David Hildenbrand 2019-07-25 11:36 ` [Qemu-devel] [PATCH-for-4.1 v4 4/7] virtio-balloon: Better names for offset variables in inflate/deflate code David Hildenbrand 2019-07-25 11:36 ` [Qemu-devel] [PATCH-for-4.1 v4 5/7] virtio-balloon: Rework pbp tracking data David Hildenbrand 2019-07-26 8:08 ` David Gibson 2019-07-25 11:36 ` [Qemu-devel] [PATCH-for-4.1 v4 6/7] virtio-balloon: Use temporary PBP only David Hildenbrand 2019-07-25 11:53 ` Michael S. Tsirkin 2019-07-25 11:56 ` David Hildenbrand 2019-07-25 11:36 ` [Qemu-devel] [PATCH-for-4.1 v4 7/7] virtio-balloon: No need to track subpages for the PBP anymore David Hildenbrand 2019-07-25 15:32 ` [Qemu-devel] [PULL 11/12] virtio-balloon: don't track subpages for the PBP Michael S. Tsirkin 2019-07-26 8:10 ` [Qemu-devel] [PATCH-for-4.1 v4 7/7] virtio-balloon: No need to track subpages for the PBP anymore David Gibson 2019-07-25 15:31 [Qemu-devel] [PULL 00/12] virtio, pc: fixes, cleanups Michael S. Tsirkin 2019-06-02 11:42 ` [Qemu-devel] [PATCH] ioapic: kvm: Skip route updates for masked pins Jan Kiszka 2019-06-02 12:10 ` Peter Xu 2019-06-03 6:30 ` Jan Kiszka 2019-06-03 0:36 ` Michael S. Tsirkin 2019-07-21 8:58 ` Jan Kiszka 2019-07-21 10:04 ` Michael S. Tsirkin 2019-07-21 16:55 ` Paolo Bonzini 2019-07-25 15:31 ` [Qemu-devel] [PULL 03/12] " Michael S. Tsirkin 2019-06-24 9:13 ` [Qemu-devel] [PATCH] docs: clarify multiqueue vs multiple virtqueues Stefan Hajnoczi 2019-06-24 10:19 ` Marc-André Lureau 2019-07-17 10:14 ` Stefan Hajnoczi 2019-07-17 10:35 ` Michael S. Tsirkin 2019-07-25 15:31 ` [Qemu-devel] [PULL 01/12] " Michael S. Tsirkin 2019-07-18 16:14 ` [Qemu-devel] [PATCH v2] i386/acpi: fix gint overflow in crs_range_compare Evgeny Yakovlev 2019-07-18 20:30 ` Michael S. Tsirkin 2019-07-25 15:31 ` [Qemu-devel] [PULL 02/12] " Michael S. Tsirkin 2019-07-19 8:54 ` [Qemu-devel] [PATCH] i386/acpi: show PCI Express bus on pxb-pcie expanders Evgeny Yakovlev 2019-07-19 12:14 ` Igor Mammedov 2019-07-25 15:31 ` [Qemu-devel] [PULL 04/12] " Michael S. Tsirkin 2019-07-25 15:32 ` [Qemu-devel] [PULL 12/12] virtio-balloon: free pbp more aggressively Michael S. Tsirkin 2019-07-26 9:53 ` [Qemu-devel] [PULL 00/12] virtio, pc: fixes, cleanups Peter Maydell
Reply instructions: You may reply publicly to this message via plain-text email using any one of the following methods: * Save the following mbox file, import it into your mail client, and reply-to-all from there: mbox Avoid top-posting and favor interleaved quoting: https://en.wikipedia.org/wiki/Posting_style#Interleaved_style * Reply using the --to, --cc, and --in-reply-to switches of git-send-email(1): git send-email \ --in-reply-to=20190722134108.22151-6-david@redhat.com \ --to=david@redhat.com \ --cc=david@gibson.dropbear.id.au \ --cc=imammedo@redhat.com \ --cc=mst@redhat.com \ --cc=qemu-devel@nongnu.org \ --cc=qemu-stable@nongnu.org \ --cc=stefanha@redhat.com \ /path/to/YOUR_REPLY https://kernel.org/pub/software/scm/git/docs/git-send-email.html * If your mail client supports setting the In-Reply-To header via mailto: links, try the mailto: linkBe sure your reply has a Subject: header at the top and a blank line before the message body.
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).