[PATCH v2] vmstate: validate VMStateDescription::fields upon registration

[PATCH v2] vmstate: validate VMStateDescription::fields upon registration

[Roman Kiryanov]

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

This change prevents this situation from happening
by explicitly crashing earlier.

Suggested-by: Fabiano Rosas <farosas@suse.de>
Suggested-by: Peter Xu <peterx@redhat.com>
Signed-off-by: Roman Kiryanov <rkir@google.com>
---
v2:
 - Moved the assert from vmstate_save_state_v to the registration
   path (vmstate_register_with_alias_id) and the qtest validation path
   (vmstate_check) to catch missing fields earlier.
---
 migration/savevm.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/migration/savevm.c b/migration/savevm.c
index 197c89e0e6..20c2b99231 100644
--- a/migration/savevm.c
+++ b/migration/savevm.c
@@ -861,6 +861,8 @@ static void vmstate_check(const VMStateDescription *vmsd)
     const VMStateField *field = vmsd->fields;
     const VMStateDescription * const *subsection = vmsd->subsections;
 
+    assert(field || vmsd->unmigratable);
+
     if (field) {
         while (field->name) {
             if (field->flags & (VMS_STRUCT | VMS_VSTRUCT)) {
@@ -900,6 +902,9 @@ int vmstate_register_with_alias_id(VMStateIf *obj, uint32_t instance_id,
     /* If this triggers, alias support can be dropped for the vmsd. */
     assert(alias_id == -1 || required_for_version >= vmsd->minimum_version_id);
 
+    /* If vmsd is migratable it MUST have valid fields. */
+    assert(vmsd->fields || vmsd->unmigratable);
+
     se = g_new0(SaveStateEntry, 1);
     se->version_id = vmsd->version_id;
     se->section_id = savevm_state.global_section_id++;
-- 
2.53.0.851.ga537e3e6e9-goog

Re: [PATCH v2] vmstate: validate VMStateDescription::fields upon registration

[Peter Xu]

On Thu, Mar 12, 2026 at 09:08:02PM +0000, Roman Kiryanov wrote:
> The vmstate_save_state_v() function does not support
> NULL in VMStateDescription::fields and will crash if
> one is provided.
> 
> This change prevents this situation from happening
> by explicitly crashing earlier.
> 
> Suggested-by: Fabiano Rosas <farosas@suse.de>
> Suggested-by: Peter Xu <peterx@redhat.com>

Thanks for the generosity, all kudos to Fabiano.  Let's replace this line
with:

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

> Signed-off-by: Roman Kiryanov <rkir@google.com>
> ---
> v2:
>  - Moved the assert from vmstate_save_state_v to the registration
>    path (vmstate_register_with_alias_id) and the qtest validation path
>    (vmstate_check) to catch missing fields earlier.
> ---
>  migration/savevm.c | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/migration/savevm.c b/migration/savevm.c
> index 197c89e0e6..20c2b99231 100644
> --- a/migration/savevm.c
> +++ b/migration/savevm.c
> @@ -861,6 +861,8 @@ static void vmstate_check(const VMStateDescription *vmsd)
>      const VMStateField *field = vmsd->fields;
>      const VMStateDescription * const *subsection = vmsd->subsections;
>  
> +    assert(field || vmsd->unmigratable);
> +
>      if (field) {
>          while (field->name) {
>              if (field->flags & (VMS_STRUCT | VMS_VSTRUCT)) {
> @@ -900,6 +902,9 @@ int vmstate_register_with_alias_id(VMStateIf *obj, uint32_t instance_id,
>      /* If this triggers, alias support can be dropped for the vmsd. */
>      assert(alias_id == -1 || required_for_version >= vmsd->minimum_version_id);
>  
> +    /* If vmsd is migratable it MUST have valid fields. */
> +    assert(vmsd->fields || vmsd->unmigratable);
> +
>      se = g_new0(SaveStateEntry, 1);
>      se->version_id = vmsd->version_id;
>      se->section_id = savevm_state.global_section_id++;
> -- 
> 2.53.0.851.ga537e3e6e9-goog
> 

-- 
Peter Xu

Re: [PATCH v2] vmstate: validate VMStateDescription::fields upon registration

[Roman Kiryanov]

On Fri, Mar 13, 2026 at 7:30 AM Peter Xu <peterx@redhat.com> wrote:
>
> Thanks for the generosity, all kudos to Fabiano.  Let's replace this line
> with:
>
> Reviewed-by: Peter Xu <peterx@redhat.com>

Sure, I will update the patch. I also noticed VirtioDeviceClass is not
covered by v2, so I added another assert into virtio_device_realize.
Now virtio-snd hits the assert if I remove unmigratable = 1.