On Wed, Jul 17, 2019 at 12:35:48PM +0200, David Hildenbrand wrote: > We are using the wrong functions to set/clear bits, effectively touching > multiple bits, writing out of range of the bitmap, resulting in memory > corruptions. We have to use set_bit()/clear_bit() instead. > > Can easily be reproduced by starting a qemu guest on hugetlbfs memory, > inflating the balloon. QEMU crashes. This never could have worked > properly - especially, also pages would have been discarded when the > first sub-page would be inflated (the whole bitmap would be set). > > While testing I realized, that on hugetlbfs it is pretty much impossible > to discard a page - the guest just frees the 4k sub-pages in random order > most of the time. I was only able to discard a hugepage a handful of > times - so I hope that now works correctly. > > 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-sta...@nongnu.org #v4.0.0 > Cc: Stefan Hajnoczi <stefa...@redhat.com> > Cc: David Gibson <da...@gibson.dropbear.id.au> > Cc: Michael S. Tsirkin <m...@redhat.com> > Cc: Igor Mammedov <imamm...@redhat.com> > Signed-off-by: David Hildenbrand <da...@redhat.com>
Ahem. You can pass me the brown paper bag now. Acked-by: David Gibson <da...@gibson.dropbear.id.au> > --- > hw/virtio/virtio-balloon.c | 10 ++++------ > 1 file changed, 4 insertions(+), 6 deletions(-) > > diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c > index e85d1c0d5c..669067d661 100644 > --- a/hw/virtio/virtio-balloon.c > +++ b/hw/virtio/virtio-balloon.c > @@ -94,9 +94,8 @@ static void balloon_inflate_page(VirtIOBalloon *balloon, > balloon->pbp->base = host_page_base; > } > > - bitmap_set(balloon->pbp->bitmap, > - (ram_offset - balloon->pbp->base) / BALLOON_PAGE_SIZE, > - subpages); > + set_bit((ram_offset - balloon->pbp->base) / BALLOON_PAGE_SIZE, > + balloon->pbp->bitmap); > > if (bitmap_full(balloon->pbp->bitmap, subpages)) { > /* We've accumulated a full host page, we can actually discard > @@ -140,9 +139,8 @@ static void balloon_deflate_page(VirtIOBalloon *balloon, > * for a guest to do this in practice, but handle it anyway, > * since getting it wrong could mean discarding memory the > * guest is still using. */ > - bitmap_clear(balloon->pbp->bitmap, > - (ram_offset - balloon->pbp->base) / BALLOON_PAGE_SIZE, > - subpages); > + clear_bit((ram_offset - balloon->pbp->base) / BALLOON_PAGE_SIZE, > + balloon->pbp->bitmap); > > if (bitmap_empty(balloon->pbp->bitmap, subpages)) { > g_free(balloon->pbp); -- 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
signature.asc
Description: PGP signature