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>
Signed-off-by: Arun Menon <armenon@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..ab20445c4c 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 | VMS_POINTER, \
+ .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..ccac731388 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 = pv;
+ uint32_t len = qemu_get_be32(f);
+
+ if (!byte_array) {
+ byte_array = g_byte_array_new();
+ }
+
+ 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 = pv;
+ uint32_t 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
Hi
On Mon, Apr 6, 2026 at 3:53 PM Arun Menon <armenon@redhat.com> 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.
Looks ok to me, although it's easy to miss something here given all
the casting, compatibility, etc.
My understanding is that this VMState code should safely replace the
existing vdagent bytearray struct (done in the next patch):
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()
}
};
some testing before<->after seems to work,
>
> Signed-off-by: Arun Menon <armenon@redhat.com>
> Fix-Suggested-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> Signed-off-by: Arun Menon <armenon@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..ab20445c4c 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 | VMS_POINTER, \
> + .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..ccac731388 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 = pv;
> + uint32_t len = qemu_get_be32(f);
> +
> + if (!byte_array) {
> + byte_array = g_byte_array_new();
You'd need to update *pv too (*(void **)pv = .. or something)
> + }
> +
> + 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 = pv;
> + uint32_t 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
>
Hi Marc-André,
Thanks for the review.
On Tue, Apr 07, 2026 at 11:32:24AM +0400, Marc-André Lureau wrote:
> Hi
>
> On Mon, Apr 6, 2026 at 3:53 PM Arun Menon <armenon@redhat.com> 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.
>
> Looks ok to me, although it's easy to miss something here given all
> the casting, compatibility, etc.
Yes, looks like I missed it. In order to update the device state
itself, I will have to pass a pointer to GByteArray pointer.
struct CRBState
{
GByteArray *command_buffer;
};
Currently VMS_POINTER flag does the job of dereferencing and then it
passes the value in pv. My bad, I misunderstood it. I am removing it,
and I shall change the get and put functions.
>
> My understanding is that this VMState code should safely replace the
> existing vdagent bytearray struct (done in the next patch):
>
> 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()
> }
> };
>
> some testing before<->after seems to work,
>
> >
> > Signed-off-by: Arun Menon <armenon@redhat.com>
> > Fix-Suggested-by: Marc-André Lureau <marcandre.lureau@redhat.com>
> > Signed-off-by: Arun Menon <armenon@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..ab20445c4c 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 | VMS_POINTER, \
> > + .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..ccac731388 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 = pv;
> > + uint32_t len = qemu_get_be32(f);
> > +
> > + if (!byte_array) {
> > + byte_array = g_byte_array_new();
>
> You'd need to update *pv too (*(void **)pv = .. or something)
Yes, changed in v2.
>
> > + }
> > +
> > + 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 = pv;
> > + uint32_t 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
> >
>
Regards,
Arun Menon
© 2016 - 2026 Red Hat, Inc.