[PATCH] vmstate: assert that VMStateDescription::fields is not NULL

[PATCH] vmstate: assert that VMStateDescription::fields is not NULL

[Roman Kiryanov]

The vmstate_save_state_v() function does not
support NULL in VMStateDescription::fields
and will crash if one is provided.

Signed-off-by: Roman Kiryanov <rkir@google.com>
---
 migration/vmstate.c | 1 +
 1 file changed, 1 insertion(+)

diff --git a/migration/vmstate.c b/migration/vmstate.c
index 4d28364f7b..5cb173ea25 100644
--- a/migration/vmstate.c
+++ b/migration/vmstate.c
@@ -433,6 +433,7 @@ int vmstate_save_state_v(QEMUFile *f, const VMStateDescription *vmsd,
     ERRP_GUARD();
     int ret = 0;
     const VMStateField *field = vmsd->fields;
+    assert(field);
 
     trace_vmstate_save_state_top(vmsd->name);
 
-- 
2.53.0.851.ga537e3e6e9-goog

Re: [PATCH] vmstate: assert that VMStateDescription::fields is not NULL

[Peter Xu]

On Mon, Mar 09, 2026 at 10:31:16PM +0000, Roman Kiryanov wrote:
> The vmstate_save_state_v() function does not
> support NULL in VMStateDescription::fields
> and will crash if one is provided.
> 
> Signed-off-by: Roman Kiryanov <rkir@google.com>

Thanks for the patch.

Yeah I think assert it is fine, but maybe unnecessary, because we have a
lot of such in QEMU (and IIUC in most userspace apps, likely even kernel is
the same..) where we assert by directly reference it as a pointer..

Here, the assert() only helps to crash slightly earlier, rather than the
field->name reference later..  While in both cases it'll be crystal clear
on what has happened when a QEMU coredump is generated, either at assert(),
or a few instructions later.

PS: please always copy Fabiano when sending migration patches.

Thanks,

> ---
>  migration/vmstate.c | 1 +
>  1 file changed, 1 insertion(+)
> 
> diff --git a/migration/vmstate.c b/migration/vmstate.c
> index 4d28364f7b..5cb173ea25 100644
> --- a/migration/vmstate.c
> +++ b/migration/vmstate.c
> @@ -433,6 +433,7 @@ int vmstate_save_state_v(QEMUFile *f, const VMStateDescription *vmsd,
>      ERRP_GUARD();
>      int ret = 0;
>      const VMStateField *field = vmsd->fields;
> +    assert(field);
>  
>      trace_vmstate_save_state_top(vmsd->name);
>  
> -- 
> 2.53.0.851.ga537e3e6e9-goog
> 

-- 
Peter Xu

Re: [PATCH] vmstate: assert that VMStateDescription::fields is not NULL

[Fabiano Rosas]

Peter Xu <peterx@redhat.com> writes:

> On Mon, Mar 09, 2026 at 10:31:16PM +0000, Roman Kiryanov wrote:
>> The vmstate_save_state_v() function does not
>> support NULL in VMStateDescription::fields
>> and will crash if one is provided.
>> 
>> Signed-off-by: Roman Kiryanov <rkir@google.com>
>
> Thanks for the patch.
>
> Yeah I think assert it is fine, but maybe unnecessary, because we have a
> lot of such in QEMU (and IIUC in most userspace apps, likely even kernel is
> the same..) where we assert by directly reference it as a pointer..
>
> Here, the assert() only helps to crash slightly earlier, rather than the
> field->name reference later..  While in both cases it'll be crystal clear
> on what has happened when a QEMU coredump is generated, either at assert(),
> or a few instructions later.
>
> PS: please always copy Fabiano when sending migration patches.
>

I'd rather have a (vmsd->fields || vmsd->unmigratable) check when
registering.

Re: [PATCH] vmstate: assert that VMStateDescription::fields is not NULL

[Roman Kiryanov]

On Tue, Mar 10, 2026 at 11:44 AM Fabiano Rosas <farosas@suse.de> wrote:
> Peter Xu <peterx@redhat.com> writes:
> > Here, the assert() only helps to crash slightly earlier, rather than the
> > field->name reference later..
>
> I'd rather have a (vmsd->fields || vmsd->unmigratable) check when
> registering.

Hi Peter and Fabiano, thank you for looking into this.

Yes, I agree that validating VMStateDescription upon registration is a
much better idea. I will send an updated patch.