[PATCH] virtio-balloon: fix free-page BH teardown on unrealize

Michael S. Tsirkin posted 1 patch 2 months ago
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/qemu tags/patchew/0967d574d9ffe469edf5fc16da86237e54a69c34.1785327742.git.mst@redhat.com
Maintainers: "Michael S. Tsirkin" <mst@redhat.com>, David Hildenbrand <david@kernel.org>
include/hw/virtio/virtio-balloon.h |  1 +
hw/virtio/virtio-balloon.c         | 19 ++++++++++++++++++-
2 files changed, 19 insertions(+), 1 deletion(-)
[PATCH] virtio-balloon: fix free-page BH teardown on unrealize
Posted by Michael S. Tsirkin 2 months ago
When a guest triggers PCIe hot-unplug while the free-page-hint BH
is running on an IOThread, the BH will keep accessing
vq->used_elems array when running it. Note that qemu_bh_delete
merely prevents new BHs from running, it does not wait
for already running ones to finish.

We need to wait for it to finish - do it like virtio scsi and
run a dummy oneshot AIO in the same context, and wait for it.

But there's a twist: BH could be blocked in qemu_cond_wait,
then AIO won't run.

Add a special reporting state FREE_PAGE_HINT_S_UNREALIZE to make BH exit
immediately.

Cc: David Hildenbrand <david@kernel.org>
Fixes: CVE-2026-66899
Resolves: https://gitlab.com/qemu-project/qemu/-/work_items/4079
Reported-by: mhun512 <mhun512@gmail.com>
Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 include/hw/virtio/virtio-balloon.h |  1 +
 hw/virtio/virtio-balloon.c         | 19 ++++++++++++++++++-
 2 files changed, 19 insertions(+), 1 deletion(-)

diff --git a/include/hw/virtio/virtio-balloon.h b/include/hw/virtio/virtio-balloon.h
index abbf339718..53bbaa4c9a 100644
--- a/include/hw/virtio/virtio-balloon.h
+++ b/include/hw/virtio/virtio-balloon.h
@@ -39,6 +39,7 @@ enum virtio_balloon_free_page_hint_status {
     FREE_PAGE_HINT_S_REQUESTED = 1,
     FREE_PAGE_HINT_S_START = 2,
     FREE_PAGE_HINT_S_DONE = 3,
+    FREE_PAGE_HINT_S_UNREALIZE = 4,
 };
 
 struct VirtIOBalloon {
diff --git a/hw/virtio/virtio-balloon.c b/hw/virtio/virtio-balloon.c
index 4c5f486ba2..07d519cf6d 100644
--- a/hw/virtio/virtio-balloon.c
+++ b/hw/virtio/virtio-balloon.c
@@ -35,6 +35,7 @@
 #include "system/reset.h"
 #include "hw/virtio/virtio-bus.h"
 #include "hw/virtio/virtio-access.h"
+#include "qemu/aio-wait.h"
 
 #define BALLOON_PAGE_SIZE  (1 << VIRTIO_BALLOON_PFN_SHIFT)
 
@@ -519,6 +520,9 @@ static bool get_free_page_hints(VirtIOBalloon *dev)
     int i;
 
     while (dev->block_iothread) {
+        if (dev->free_page_hint_status == FREE_PAGE_HINT_S_UNREALIZE) {
+            return false;
+        }
         qemu_cond_wait(&dev->free_page_cond, &dev->free_page_lock);
     }
 
@@ -912,6 +916,11 @@ static void virtio_balloon_device_realize(DeviceState *dev, Error **errp)
     qemu_register_resettable(OBJECT(dev));
 }
 
+static void dummy_bh(void *opaque)
+{
+    /* Do nothing */
+}
+
 static void virtio_balloon_device_unrealize(DeviceState *dev)
 {
     VirtIODevice *vdev = VIRTIO_DEVICE(dev);
@@ -919,9 +928,17 @@ static void virtio_balloon_device_unrealize(DeviceState *dev)
 
     qemu_unregister_resettable(OBJECT(dev));
     if (s->free_page_bh) {
+        AioContext *ctx = iothread_get_aio_context(s->iothread);
+
         qemu_bh_delete(s->free_page_bh);
+
+        qemu_mutex_lock(&s->free_page_lock);
+        s->free_page_hint_status = FREE_PAGE_HINT_S_UNREALIZE;
+        qemu_cond_signal(&s->free_page_cond);
+        qemu_mutex_unlock(&s->free_page_lock);
+        aio_wait_bh_oneshot(ctx, dummy_bh, NULL);
+
         object_unref(OBJECT(s->iothread));
-        virtio_balloon_free_page_stop(s);
         precopy_remove_notifier(&s->free_page_hint_notify);
     }
     balloon_stats_destroy_timer(s);
-- 
MST
Re: [PATCH] virtio-balloon: fix free-page BH teardown on unrealize
Posted by David Hildenbrand (Arm) 1 month, 4 weeks ago
> +
>  static void virtio_balloon_device_unrealize(DeviceState *dev)
>  {
>      VirtIODevice *vdev = VIRTIO_DEVICE(dev);
> @@ -919,9 +928,17 @@ static void virtio_balloon_device_unrealize(DeviceState *dev)
>  
>      qemu_unregister_resettable(OBJECT(dev));
>      if (s->free_page_bh) {
> +        AioContext *ctx = iothread_get_aio_context(s->iothread);
> +
>          qemu_bh_delete(s->free_page_bh);
> +
> +        qemu_mutex_lock(&s->free_page_lock);
> +        s->free_page_hint_status = FREE_PAGE_HINT_S_UNREALIZE;
> +        qemu_cond_signal(&s->free_page_cond);
> +        qemu_mutex_unlock(&s->free_page_lock);
> +        aio_wait_bh_oneshot(ctx, dummy_bh, NULL);
> +
>          object_unref(OBJECT(s->iothread));
> -        virtio_balloon_free_page_stop(s);
>          precopy_remove_notifier(&s->free_page_hint_notify);
>      }
>      balloon_stats_destroy_timer(s);

Not sure if this already went upsteam, but FWIW

Reviewed-by: David Hildenbrand <david@kernel.org>

-- 
Cheers,

David