[PATCH 1/2] vhost-user-base: free virtqueue array during cleanup

[PATCH 1/2] vhost-user-base: free virtqueue array during cleanup

[zhaoguohan]

From: GuoHan Zhao <zhaoguohan@kylinos.cn>

vhost-user-base stores the VirtQueue pointers in a GPtrArray, but its
cleanup helper only deletes the VirtQueues and leaves the array itself
allocated.

Free the GPtrArray after deleting the queues and clear the pointer so
cleanup remains safe if the error path reaches it with no queues to
release.

Fixes: 6275989647ef (virtio: split into vhost-user-base and vhost-user-device)
Signed-off-by: GuoHan Zhao <zhaoguohan@kylinos.cn>
---
 hw/virtio/vhost-user-base.c | 10 +++++++---
 1 file changed, 7 insertions(+), 3 deletions(-)

diff --git a/hw/virtio/vhost-user-base.c b/hw/virtio/vhost-user-base.c
index 39b5e637fc37..6ac523f9ebef 100644
--- a/hw/virtio/vhost-user-base.c
+++ b/hw/virtio/vhost-user-base.c
@@ -193,9 +193,13 @@ static void do_vhost_user_cleanup(VirtIODevice *vdev, VHostUserBase *vub)
 {
     vhost_user_cleanup(&vub->vhost_user);
 
-    for (int i = 0; i < vub->num_vqs; i++) {
-        VirtQueue *vq = g_ptr_array_index(vub->vqs, i);
-        virtio_delete_queue(vq);
+    if (vub->vqs) {
+        for (int i = 0; i < vub->num_vqs; i++) {
+            VirtQueue *vq = g_ptr_array_index(vub->vqs, i);
+            virtio_delete_queue(vq);
+        }
+        g_ptr_array_free(vub->vqs, true);
+        vub->vqs = NULL;
     }
 
     virtio_cleanup(vdev);
-- 
2.43.0

[PATCH 2/2] vhost-user-base: clean up vhost_dev on realize failure

[zhaoguohan]

From: GuoHan Zhao <zhaoguohan@kylinos.cn>

vhost-user-base queries the backend shared memory configuration after
vhost_dev_init() succeeds. If that query fails, or if the shared memory
configuration fails validation, realization jumps to the error path
without cleaning up the initialized vhost_dev.

Track whether vhost_dev_init() succeeded and call vhost_dev_cleanup()
from the error path in that case. Keep a copy of vhost_dev.vqs so the
allocation can still be freed after vhost_dev_cleanup() clears struct
vhost_dev.

Fixes: 6608dca74ecf (vhost-user-device: Add shared memory BAR)
Signed-off-by: GuoHan Zhao <zhaoguohan@kylinos.cn>
---
 hw/virtio/vhost-user-base.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/hw/virtio/vhost-user-base.c b/hw/virtio/vhost-user-base.c
index 6ac523f9ebef..000e91d50331 100644
--- a/hw/virtio/vhost-user-base.c
+++ b/hw/virtio/vhost-user-base.c
@@ -287,6 +287,8 @@ static void vub_device_realize(DeviceState *dev, Error **errp)
     VirtIODevice *vdev = VIRTIO_DEVICE(dev);
     VHostUserBase *vub = VHOST_USER_BASE(dev);
     uint64_t memory_sizes[VIRTIO_MAX_SHMEM_REGIONS];
+    struct vhost_virtqueue *vhost_vqs = NULL;
+    bool vhost_dev_initialized = false;
     int i, ret, nregions, regions_processed = 0;
 
     if (!vub->chardev.chr) {
@@ -338,6 +340,7 @@ static void vub_device_realize(DeviceState *dev, Error **errp)
 
     vub->vhost_dev.nvqs = vub->num_vqs;
     vub->vhost_dev.vqs = g_new0(struct vhost_virtqueue, vub->vhost_dev.nvqs);
+    vhost_vqs = vub->vhost_dev.vqs;
 
     /* connect to backend */
     ret = vhost_dev_init(&vub->vhost_dev, &vub->vhost_user,
@@ -346,6 +349,7 @@ static void vub_device_realize(DeviceState *dev, Error **errp)
     if (ret < 0) {
         goto err;
     }
+    vhost_dev_initialized = true;
 
     ret = vub->vhost_dev.vhost_ops->vhost_get_shmem_config(&vub->vhost_dev,
                                                            &nregions,
@@ -386,6 +390,10 @@ static void vub_device_realize(DeviceState *dev, Error **errp)
                              dev, NULL, true);
     return;
 err:
+    if (vhost_dev_initialized) {
+        vhost_dev_cleanup(&vub->vhost_dev);
+    }
+    g_free(vhost_vqs);
     do_vhost_user_cleanup(vdev, vub);
 }
 
-- 
2.43.0

Re: [PATCH 2/2] vhost-user-base: clean up vhost_dev on realize failure

[Michael S. Tsirkin]

On Mon, Jun 29, 2026 at 05:26:19PM +0800, zhaoguohan@kylinos.cn wrote:
> From: GuoHan Zhao <zhaoguohan@kylinos.cn>
> 
> vhost-user-base queries the backend shared memory configuration after
> vhost_dev_init() succeeds. If that query fails, or if the shared memory
> configuration fails validation, realization jumps to the error path
> without cleaning up the initialized vhost_dev.
> 
> Track whether vhost_dev_init() succeeded and call vhost_dev_cleanup()
> from the error path in that case. Keep a copy of vhost_dev.vqs so the
> allocation can still be freed after vhost_dev_cleanup() clears struct
> vhost_dev.
> 
> Fixes: 6608dca74ecf (vhost-user-device: Add shared memory BAR)
> Signed-off-by: GuoHan Zhao <zhaoguohan@kylinos.cn>
> ---
>  hw/virtio/vhost-user-base.c | 8 ++++++++
>  1 file changed, 8 insertions(+)
> 
> diff --git a/hw/virtio/vhost-user-base.c b/hw/virtio/vhost-user-base.c
> index 6ac523f9ebef..000e91d50331 100644
> --- a/hw/virtio/vhost-user-base.c
> +++ b/hw/virtio/vhost-user-base.c
> @@ -287,6 +287,8 @@ static void vub_device_realize(DeviceState *dev, Error **errp)
>      VirtIODevice *vdev = VIRTIO_DEVICE(dev);
>      VHostUserBase *vub = VHOST_USER_BASE(dev);
>      uint64_t memory_sizes[VIRTIO_MAX_SHMEM_REGIONS];
> +    struct vhost_virtqueue *vhost_vqs = NULL;
> +    bool vhost_dev_initialized = false;
>      int i, ret, nregions, regions_processed = 0;
>  
>      if (!vub->chardev.chr) {
> @@ -338,6 +340,7 @@ static void vub_device_realize(DeviceState *dev, Error **errp)
>  
>      vub->vhost_dev.nvqs = vub->num_vqs;
>      vub->vhost_dev.vqs = g_new0(struct vhost_virtqueue, vub->vhost_dev.nvqs);
> +    vhost_vqs = vub->vhost_dev.vqs;
>  
>      /* connect to backend */
>      ret = vhost_dev_init(&vub->vhost_dev, &vub->vhost_user,
> @@ -346,6 +349,7 @@ static void vub_device_realize(DeviceState *dev, Error **errp)
>      if (ret < 0) {
>          goto err;
>      }
> +    vhost_dev_initialized = true;
>  
>      ret = vub->vhost_dev.vhost_ops->vhost_get_shmem_config(&vub->vhost_dev,
>                                                             &nregions,
> @@ -386,6 +390,10 @@ static void vub_device_realize(DeviceState *dev, Error **errp)
>                               dev, NULL, true);
>      return;
>  err:
> +    if (vhost_dev_initialized) {
> +        vhost_dev_cleanup(&vub->vhost_dev);
> +    }
> +    g_free(vhost_vqs);
>      do_vhost_user_cleanup(vdev, vub);
>  }

Pls just add another label.


> -- 
> 2.43.0

[PATCH v2 2/2] vhost-user-base: clean up vhost_dev on realize failure

[zhaoguohan]

From: GuoHan Zhao <zhaoguohan@kylinos.cn>

Failures after vhost_dev_init() currently skip vhost_dev_cleanup(),
leaking initialized vhost_dev state.

Add a separate unwind label for those paths. Keep a copy of
vhost_dev.vqs so the array can still be freed after vhost_dev_cleanup()
clears struct vhost_dev.

Fixes: 6608dca74ecf (vhost-user-device: Add shared memory BAR)
Signed-off-by: GuoHan Zhao <zhaoguohan@kylinos.cn>
---
v2:
- Use a separate error label instead of a bool.

 hw/virtio/vhost-user-base.c | 11 ++++++++---
 1 file changed, 8 insertions(+), 3 deletions(-)

diff --git a/hw/virtio/vhost-user-base.c b/hw/virtio/vhost-user-base.c
index 39b5e637fc37..62a139ebbdfe 100644
--- a/hw/virtio/vhost-user-base.c
+++ b/hw/virtio/vhost-user-base.c
@@ -283,6 +283,7 @@ static void vub_device_realize(DeviceState *dev, Error **errp)
     VirtIODevice *vdev = VIRTIO_DEVICE(dev);
     VHostUserBase *vub = VHOST_USER_BASE(dev);
     uint64_t memory_sizes[VIRTIO_MAX_SHMEM_REGIONS];
+    struct vhost_virtqueue *vhost_vqs = NULL;
     int i, ret, nregions, regions_processed = 0;
 
     if (!vub->chardev.chr) {
@@ -334,6 +335,7 @@ static void vub_device_realize(DeviceState *dev, Error **errp)
 
     vub->vhost_dev.nvqs = vub->num_vqs;
     vub->vhost_dev.vqs = g_new0(struct vhost_virtqueue, vub->vhost_dev.nvqs);
+    vhost_vqs = vub->vhost_dev.vqs;
 
     /* connect to backend */
     ret = vhost_dev_init(&vub->vhost_dev, &vub->vhost_user,
@@ -349,7 +351,7 @@ static void vub_device_realize(DeviceState *dev, Error **errp)
                                                            errp);
 
     if (ret < 0) {
-        goto err;
+        goto err_vhost_dev;
     }
 
     for (i = 0; i < VIRTIO_MAX_SHMEM_REGIONS && regions_processed < nregions; i++) {
@@ -364,14 +366,14 @@ static void vub_device_realize(DeviceState *dev, Error **errp)
                     errp);
 
                 if (ret < 0) {
-                    goto err;
+                    goto err_vhost_dev;
                 }
             }
 
             if (memory_sizes[i] % qemu_real_host_page_size() != 0) {
                 error_setg(errp, "Shared memory %d size must be a multiple of "
                                  "the host page size", i);
-                goto err;
+                goto err_vhost_dev;
             }
 
             virtio_new_shmem_region(vdev, i, memory_sizes[i]);
@@ -381,7 +383,10 @@ static void vub_device_realize(DeviceState *dev, Error **errp)
     qemu_chr_fe_set_handlers(&vub->chardev, NULL, NULL, vub_event, NULL,
                              dev, NULL, true);
     return;
+err_vhost_dev:
+    vhost_dev_cleanup(&vub->vhost_dev);
 err:
+    g_free(vhost_vqs);
     do_vhost_user_cleanup(vdev, vub);
 }
 
-- 
2.43.0