[PATCH v3 0/2] migration/vmstate: Add VMState support to safely migrate GByteArray

[PATCH v3 0/2] migration/vmstate: Add VMState support to safely migrate GByteArray

[Arun Menon]

In GLib, GByteArray is an object managed by the library. Currently,
migrating a GByteArray requires treating it as a raw C struct and using
VMSTATE_VBUFFER_ALLOC_UINT32. For example, see vmstate_vdba in
ui/vdagent.c

QEMU cannot pretend that GByteArray is a C struct and simply use
VMS_ALLOC to g_malloc() the buffer. This is because, VMS_ALLOC blindly
overwrites the data pointer with a newly allocated buffer, thereby
leaking the previous memory. Besides, GLib tracks the array's capacity
in a hidden alloc field. Bypassing GLib APIs leave this capacity out of
sync with the newly allocated buffer, potentially leading to heap buffer
overflows during subsequent g_byte_array_append() calls.

This series adds a new VMState called VMSTATE_GBYTEARRAY, which will
directly use GLib library API calls to create, or resize the object.
This is then used in ui/vdaagent.c replacing the use of
VMSTATE_VBUFFER_ALLOC_UINT32.

Changes in v3:
- Add assert() checks to get and put functions to prevent NULL pointer
  dereferences, as suggested by Peter Xu.

Changes in v2:
- Marc-André pointed out the problem of not updating the device state
  itself. To do that we should essentially pass a pointer to the
  GByteArray pointer. Remove VMS_POINTER from VMSTATE_GBYTEARRAY. This
  changes 'pv' to be the address of the pointer field (GByteArray **)
  instead of the pointer's value.
- Update get_g_byte_array and put_g_byte_array to handle this additional
  level of indirection, allowing safe allocation and updating device state.
- Link to v1: https://lore.kernel.org/all/20260406115247.4879-1-armenon@redhat.com/

Arun Menon (2):
  migration/vmstate: Add VMState support for GByteArray
  ui/vdagent: Use VMSTATE_GBYTEARRAY to safely migrate outbuf

 include/migration/vmstate.h | 10 ++++++++++
 migration/vmstate-types.c   | 37 +++++++++++++++++++++++++++++++++++++
 ui/vdagent.c                | 13 +------------
 3 files changed, 48 insertions(+), 12 deletions(-)

-- 
2.53.0

[PATCH v3 1/2] migration/vmstate: Add VMState support for GByteArray

[Arun Menon]

From: Arun Menon <armenon@redhat.com>

In GLib, GByteArray is an object managed by the library. Currently,
migrating a GByteArray requires treating it as a raw C struct and using
VMSTATE_VBUFFER_ALLOC_UINT32. For example, see vmstate_vdba in
ui/vdagent.c

QEMU cannot pretend that GByteArray is a C struct and simply use
VMS_ALLOC to g_malloc() the buffer. This is because, VMS_ALLOC blindly
overwrites the data pointer with a newly allocated buffer, thereby
leaking the previous memory. Besides, GLib tracks the array's capacity
in a hidden alloc field. Bypassing GLib APIs leave this capacity out of
sync with the newly allocated buffer, potentially leading to heap buffer
overflows during subsequent g_byte_array_append() calls.

This commit introduces VMSTATE_GBYTEARRAY which uses specific library
API calls (g_byte_array_set_size()) to safely resize and populate the
buffer.

Signed-off-by: Arun Menon <armenon@redhat.com>
Fix-Suggested-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 include/migration/vmstate.h | 10 ++++++++++
 migration/vmstate-types.c   | 37 +++++++++++++++++++++++++++++++++++++
 2 files changed, 47 insertions(+)

diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
index 62c2abd0c4..f503a40ec0 100644
--- a/include/migration/vmstate.h
+++ b/include/migration/vmstate.h
@@ -265,6 +265,7 @@ extern const VMStateInfo vmstate_info_bitmap;
 extern const VMStateInfo vmstate_info_qtailq;
 extern const VMStateInfo vmstate_info_gtree;
 extern const VMStateInfo vmstate_info_qlist;
+extern const VMStateInfo vmstate_info_g_byte_array;
 
 #define type_check_2darray(t1,t2,n,m) ((t1(*)[n][m])0 - (t2*)0)
 /*
@@ -892,6 +893,15 @@ extern const VMStateInfo vmstate_info_qlist;
     .start        = offsetof(_type, _next),                              \
 }
 
+#define VMSTATE_GBYTEARRAY(_field, _state, _version) {                   \
+    .name         = (stringify(_field)),                                 \
+    .version_id   = (_version),                                          \
+    .size         = sizeof(GByteArray),                                  \
+    .info         = &vmstate_info_g_byte_array,                          \
+    .flags        = VMS_SINGLE,                                          \
+    .offset       = vmstate_offset_pointer(_state, _field, GByteArray),  \
+}
+
 /* _f : field name
    _f_n : num of elements field_name
    _n : num of elements
diff --git a/migration/vmstate-types.c b/migration/vmstate-types.c
index 89cb211472..e3a47e60b1 100644
--- a/migration/vmstate-types.c
+++ b/migration/vmstate-types.c
@@ -942,3 +942,40 @@ const VMStateInfo vmstate_info_qlist = {
     .get  = get_qlist,
     .put  = put_qlist,
 };
+
+static int get_g_byte_array(QEMUFile *f, void *pv, size_t size,
+                            const VMStateField *field)
+{
+    GByteArray **byte_array = (GByteArray **)pv;
+    uint32_t len = qemu_get_be32(f);
+
+    assert(*byte_array != NULL);
+
+    g_byte_array_set_size(*byte_array, len);
+    if (len > 0) {
+        qemu_get_buffer(f, (*byte_array)->data, len);
+    }
+    return 0;
+}
+
+static int put_g_byte_array(QEMUFile *f, void *pv, size_t size,
+                            const VMStateField *field, JSONWriter *vmdesc)
+{
+    GByteArray *byte_array = *(GByteArray **)pv;
+    uint32_t len;
+    assert(byte_array != NULL);
+
+    len = byte_array ? byte_array->len : 0;
+    qemu_put_be32(f, len);
+    if (len > 0) {
+        qemu_put_buffer(f, byte_array->data, len);
+    }
+
+    return 0;
+}
+
+const VMStateInfo vmstate_info_g_byte_array = {
+    .name = "GByteArray",
+    .get  = get_g_byte_array,
+    .put  = put_g_byte_array,
+};
-- 
2.53.0

[PATCH v3 2/2] ui/vdagent: Use VMSTATE_GBYTEARRAY to safely migrate outbuf

[Arun Menon]

From: Arun Menon <armenon@redhat.com>

Migrating a GLib GByteArray is now possible directly using the newly
introduced VMSTATE_GBYTEARRAY. It uses the standard GLib API calls to
create the array, or resize it.

This is safer than implementing a C struct and manually updating the
data and len fields. This commit uses the VMSTATE_GBYTEARRAY in vdagent
to store the outbuf variable.

Signed-off-by: Arun Menon <armenon@redhat.com>
Fix-Suggested-by: Marc-André Lureau <marcandre.lureau@redhat.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 ui/vdagent.c | 13 +------------
 1 file changed, 1 insertion(+), 12 deletions(-)

diff --git a/ui/vdagent.c b/ui/vdagent.c
index bb0c4aa14c..9206e47000 100644
--- a/ui/vdagent.c
+++ b/ui/vdagent.c
@@ -967,17 +967,6 @@ static const VMStateDescription vmstate_chunk = {
     }
 };
 
-static const VMStateDescription vmstate_vdba = {
-    .name = "vdagent/bytearray",
-    .version_id = 0,
-    .minimum_version_id = 0,
-    .fields = (const VMStateField[]) {
-        VMSTATE_UINT32(len, GByteArray),
-        VMSTATE_VBUFFER_ALLOC_UINT32(data, GByteArray, 0, 0, len),
-        VMSTATE_END_OF_LIST()
-    }
-};
-
 struct CBInfoArray {
     uint32_t n;
     QemuClipboardInfo cbinfo[QEMU_CLIPBOARD_SELECTION__COUNT];
@@ -1067,7 +1056,7 @@ static const VMStateDescription vmstate_vdagent = {
         VMSTATE_UINT32(xsize, VDAgentChardev),
         VMSTATE_UINT32(xoff, VDAgentChardev),
         VMSTATE_VBUFFER_ALLOC_UINT32(xbuf, VDAgentChardev, 0, 0, xsize),
-        VMSTATE_STRUCT_POINTER(outbuf, VDAgentChardev, vmstate_vdba, GByteArray),
+        VMSTATE_GBYTEARRAY(outbuf, VDAgentChardev, 0),
         VMSTATE_UINT32(mouse_x, VDAgentChardev),
         VMSTATE_UINT32(mouse_y, VDAgentChardev),
         VMSTATE_UINT32(mouse_btn, VDAgentChardev),
-- 
2.53.0

Re: [PATCH v3 1/2] migration/vmstate: Add VMState support for GByteArray

[Peter Xu]

On Wed, Apr 22, 2026 at 01:52:13PM +0530, Arun Menon wrote:
> From: Arun Menon <armenon@redhat.com>
> 
> In GLib, GByteArray is an object managed by the library. Currently,
> migrating a GByteArray requires treating it as a raw C struct and using
> VMSTATE_VBUFFER_ALLOC_UINT32. For example, see vmstate_vdba in
> ui/vdagent.c
> 
> QEMU cannot pretend that GByteArray is a C struct and simply use
> VMS_ALLOC to g_malloc() the buffer. This is because, VMS_ALLOC blindly
> overwrites the data pointer with a newly allocated buffer, thereby
> leaking the previous memory. Besides, GLib tracks the array's capacity
> in a hidden alloc field. Bypassing GLib APIs leave this capacity out of
> sync with the newly allocated buffer, potentially leading to heap buffer
> overflows during subsequent g_byte_array_append() calls.
> 
> This commit introduces VMSTATE_GBYTEARRAY which uses specific library
> API calls (g_byte_array_set_size()) to safely resize and populate the
> buffer.
> 
> Signed-off-by: Arun Menon <armenon@redhat.com>
> Fix-Suggested-by: Marc-André Lureau <marcandre.lureau@redhat.com>

I think the common one is Suggested-by for this tag.

> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

Reviewed-by: Peter Xu <peterx@redhat.com>

Nitpicks only inline.

> ---
>  include/migration/vmstate.h | 10 ++++++++++
>  migration/vmstate-types.c   | 37 +++++++++++++++++++++++++++++++++++++
>  2 files changed, 47 insertions(+)
> 
> diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
> index 62c2abd0c4..f503a40ec0 100644
> --- a/include/migration/vmstate.h
> +++ b/include/migration/vmstate.h
> @@ -265,6 +265,7 @@ extern const VMStateInfo vmstate_info_bitmap;
>  extern const VMStateInfo vmstate_info_qtailq;
>  extern const VMStateInfo vmstate_info_gtree;
>  extern const VMStateInfo vmstate_info_qlist;
> +extern const VMStateInfo vmstate_info_g_byte_array;
>  
>  #define type_check_2darray(t1,t2,n,m) ((t1(*)[n][m])0 - (t2*)0)
>  /*
> @@ -892,6 +893,15 @@ extern const VMStateInfo vmstate_info_qlist;
>      .start        = offsetof(_type, _next),                              \
>  }
>  
> +#define VMSTATE_GBYTEARRAY(_field, _state, _version) {                   \
> +    .name         = (stringify(_field)),                                 \
> +    .version_id   = (_version),                                          \
> +    .size         = sizeof(GByteArray),                                  \
> +    .info         = &vmstate_info_g_byte_array,                          \
> +    .flags        = VMS_SINGLE,                                          \
> +    .offset       = vmstate_offset_pointer(_state, _field, GByteArray),  \
> +}
> +
>  /* _f : field name
>     _f_n : num of elements field_name
>     _n : num of elements
> diff --git a/migration/vmstate-types.c b/migration/vmstate-types.c
> index 89cb211472..e3a47e60b1 100644
> --- a/migration/vmstate-types.c
> +++ b/migration/vmstate-types.c
> @@ -942,3 +942,40 @@ const VMStateInfo vmstate_info_qlist = {
>      .get  = get_qlist,
>      .put  = put_qlist,
>  };
> +
> +static int get_g_byte_array(QEMUFile *f, void *pv, size_t size,
> +                            const VMStateField *field)
> +{
> +    GByteArray **byte_array = (GByteArray **)pv;
> +    uint32_t len = qemu_get_be32(f);
> +
> +    assert(*byte_array != NULL);

We can make byte_array to be GByteArray* directly, it'll be the same as
assertion when deref NULL.

> +
> +    g_byte_array_set_size(*byte_array, len);
> +    if (len > 0) {
> +        qemu_get_buffer(f, (*byte_array)->data, len);
> +    }
> +    return 0;
> +}
> +
> +static int put_g_byte_array(QEMUFile *f, void *pv, size_t size,
> +                            const VMStateField *field, JSONWriter *vmdesc)
> +{
> +    GByteArray *byte_array = *(GByteArray **)pv;
> +    uint32_t len;
> +    assert(byte_array != NULL);
> +
> +    len = byte_array ? byte_array->len : 0;

This check is redundant.

> +    qemu_put_be32(f, len);
> +    if (len > 0) {
> +        qemu_put_buffer(f, byte_array->data, len);
> +    }
> +
> +    return 0;
> +}
> +
> +const VMStateInfo vmstate_info_g_byte_array = {
> +    .name = "GByteArray",
> +    .get  = get_g_byte_array,
> +    .put  = put_g_byte_array,
> +};
> -- 
> 2.53.0
> 

-- 
Peter Xu

Re: [PATCH v3 2/2] ui/vdagent: Use VMSTATE_GBYTEARRAY to safely migrate outbuf

[Peter Xu]

On Wed, Apr 22, 2026 at 01:52:14PM +0530, Arun Menon wrote:
> From: Arun Menon <armenon@redhat.com>
> 
> Migrating a GLib GByteArray is now possible directly using the newly
> introduced VMSTATE_GBYTEARRAY. It uses the standard GLib API calls to
> create the array, or resize it.
> 
> This is safer than implementing a C struct and manually updating the
> data and len fields. This commit uses the VMSTATE_GBYTEARRAY in vdagent
> to store the outbuf variable.
> 
> Signed-off-by: Arun Menon <armenon@redhat.com>
> Fix-Suggested-by: Marc-André Lureau <marcandre.lureau@redhat.com>

Same here.

> Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>

Reviewed-by: Peter Xu <peterx@redhat.com>

-- 
Peter Xu

Re: [PATCH v3 1/2] migration/vmstate: Add VMState support for GByteArray

[Arun Menon]

On Wed, Apr 22, 2026 at 03:58:41PM -0400, Peter Xu wrote:
> On Wed, Apr 22, 2026 at 01:52:13PM +0530, Arun Menon wrote:
> > From: Arun Menon <armenon@redhat.com>
> > 
> > In GLib, GByteArray is an object managed by the library. Currently,
> > migrating a GByteArray requires treating it as a raw C struct and using
> > VMSTATE_VBUFFER_ALLOC_UINT32. For example, see vmstate_vdba in
> > ui/vdagent.c
> > 
> > QEMU cannot pretend that GByteArray is a C struct and simply use
> > VMS_ALLOC to g_malloc() the buffer. This is because, VMS_ALLOC blindly
> > overwrites the data pointer with a newly allocated buffer, thereby
> > leaking the previous memory. Besides, GLib tracks the array's capacity
> > in a hidden alloc field. Bypassing GLib APIs leave this capacity out of
> > sync with the newly allocated buffer, potentially leading to heap buffer
> > overflows during subsequent g_byte_array_append() calls.
> > 
> > This commit introduces VMSTATE_GBYTEARRAY which uses specific library
> > API calls (g_byte_array_set_size()) to safely resize and populate the
> > buffer.
> > 
> > Signed-off-by: Arun Menon <armenon@redhat.com>
> > Fix-Suggested-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> I think the common one is Suggested-by for this tag.
> 
> > Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> 
> Reviewed-by: Peter Xu <peterx@redhat.com>
> 
> Nitpicks only inline.
> 
> > ---
> >  include/migration/vmstate.h | 10 ++++++++++
> >  migration/vmstate-types.c   | 37 +++++++++++++++++++++++++++++++++++++
> >  2 files changed, 47 insertions(+)
> > 
> > diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
> > index 62c2abd0c4..f503a40ec0 100644
> > --- a/include/migration/vmstate.h
> > +++ b/include/migration/vmstate.h
> > @@ -265,6 +265,7 @@ extern const VMStateInfo vmstate_info_bitmap;
> >  extern const VMStateInfo vmstate_info_qtailq;
> >  extern const VMStateInfo vmstate_info_gtree;
> >  extern const VMStateInfo vmstate_info_qlist;
> > +extern const VMStateInfo vmstate_info_g_byte_array;
> >  
> >  #define type_check_2darray(t1,t2,n,m) ((t1(*)[n][m])0 - (t2*)0)
> >  /*
> > @@ -892,6 +893,15 @@ extern const VMStateInfo vmstate_info_qlist;
> >      .start        = offsetof(_type, _next),                              \
> >  }
> >  
> > +#define VMSTATE_GBYTEARRAY(_field, _state, _version) {                   \
> > +    .name         = (stringify(_field)),                                 \
> > +    .version_id   = (_version),                                          \
> > +    .size         = sizeof(GByteArray),                                  \
> > +    .info         = &vmstate_info_g_byte_array,                          \
> > +    .flags        = VMS_SINGLE,                                          \
> > +    .offset       = vmstate_offset_pointer(_state, _field, GByteArray),  \
> > +}
> > +
> >  /* _f : field name
> >     _f_n : num of elements field_name
> >     _n : num of elements
> > diff --git a/migration/vmstate-types.c b/migration/vmstate-types.c
> > index 89cb211472..e3a47e60b1 100644
> > --- a/migration/vmstate-types.c
> > +++ b/migration/vmstate-types.c
> > @@ -942,3 +942,40 @@ const VMStateInfo vmstate_info_qlist = {
> >      .get  = get_qlist,
> >      .put  = put_qlist,
> >  };
> > +
> > +static int get_g_byte_array(QEMUFile *f, void *pv, size_t size,
> > +                            const VMStateField *field)
> > +{
> > +    GByteArray **byte_array = (GByteArray **)pv;
> > +    uint32_t len = qemu_get_be32(f);
> > +
> > +    assert(*byte_array != NULL);
> 
> We can make byte_array to be GByteArray* directly, it'll be the same as
> assertion when deref NULL.

Thanks for the feedback, Peter.
I will simplify the pointer handling in get_g_byte_array() and fix the
commit tag to Suggested-by.

Regarding the assert(), Would you like me to remove the
explicit assert() in put_g_byte_array() as well? Since it is immediately
followed by a dereference:

qemu_put_be32(f, byte_array->len);

> 
> > +
> > +    g_byte_array_set_size(*byte_array, len);
> > +    if (len > 0) {
> > +        qemu_get_buffer(f, (*byte_array)->data, len);
> > +    }
> > +    return 0;
> > +}
> > +
> > +static int put_g_byte_array(QEMUFile *f, void *pv, size_t size,
> > +                            const VMStateField *field, JSONWriter *vmdesc)
> > +{
> > +    GByteArray *byte_array = *(GByteArray **)pv;
> > +    uint32_t len;
> > +    assert(byte_array != NULL);
> > +
> > +    len = byte_array ? byte_array->len : 0;
> 
> This check is redundant.
> 
> > +    qemu_put_be32(f, len);
> > +    if (len > 0) {
> > +        qemu_put_buffer(f, byte_array->data, len);
> > +    }
> > +
> > +    return 0;
> > +}
> > +
> > +const VMStateInfo vmstate_info_g_byte_array = {
> > +    .name = "GByteArray",
> > +    .get  = get_g_byte_array,
> > +    .put  = put_g_byte_array,
> > +};
> > -- 
> > 2.53.0
> > 
> 
> -- 
> Peter Xu
> 

Regards,
Arun Menon

Re: [PATCH v3 1/2] migration/vmstate: Add VMState support for GByteArray

[Peter Xu]

On Thu, Apr 23, 2026 at 12:58:33PM +0530, Arun Menon wrote:
> Regarding the assert(), Would you like me to remove the
> explicit assert() in put_g_byte_array() as well? Since it is immediately
> followed by a dereference:
> 
> qemu_put_be32(f, byte_array->len);

Your new version looks good, thanks.

-- 
Peter Xu