On 3/10/26 10:32, Alexandr Moshkov wrote:


On 3/6/26 18:46, Fabiano Rosas wrote:
Alexandr Moshkov<[email protected]> writes:

When loading a subset, its name is checked for the parent prefix. The
following bug may occur here:

Let's say there is a vmstate named "virtio-blk", it has a subsection
named "virtio-blk/subsection", and it also has another vmstate named
"virtio" in the fields.
Then, during the migration, when trying to load this subsection for
"virtio", the prefix condition will pass for "virtio-blk/subsection" and
then the migration will break, because this vmstate does not have such a
subsection.

In other words, if a field inside vmstate1 is set via vmstate2 with a
name that is a prefix of the parent vmstate, then the field can "steal"
a subsection belonging to the parent state.

Looks like it happens because migration stream for "virtio-blk" looks
like this:

  [virtio-blk header] [virtio-blk fields] [virtio-blk subsections]

"virtio-blk" contains "virtio" field, so migration stream is:

  [virtio-blk header] [virtio header] [virtio fields] [virtio
subsections] [virtio-blk subsections]

And when we load the subsections of the "virtio" device,
vmstate_subsection_load() uses qemu_peek_byte() to try to figure out if
this is his subsection. This is where we encounter an error.

Thus, the error occurs due to the fact that vmsd does not know how many
subsections it has when loading (this does not appear anywhere in the
migration stream), so it tries to load all the appropriate ones by
names.

Fix it by checking `/` at the end of idstr.

Signed-off-by: Alexandr Moshkov<[email protected]>
---
  migration/vmstate.c | 7 +++++--
  1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/migration/vmstate.c b/migration/vmstate.c
index 4d28364f7b..187f3861f2 100644
--- a/migration/vmstate.c
+++ b/migration/vmstate.c
@@ -613,7 +613,7 @@ static int vmstate_subsection_load(QEMUFile *f, const 
VMStateDescription *vmsd,
while (qemu_peek_byte(f, 0) == QEMU_VM_SUBSECTION) {
          char idstr[256], *idstr_ret;
-        int ret;
+        int ret, vmsd_name_len;
          uint8_t version_id, len, size;
          const VMStateDescription *sub_vmsd;
@@ -631,7 +631,10 @@ static int vmstate_subsection_load(QEMUFile *f, const VMStateDescription *vmsd,
          memcpy(idstr, idstr_ret, size);
          idstr[size] = 0;
- if (strncmp(vmsd->name, idstr, strlen(vmsd->name)) != 0) {
+        vmsd_name_len = strlen(vmsd->name);
+        if (strncmp(vmsd->name, idstr, vmsd_name_len) != 0 ||
+            /* to avoid taking parent subsection here */
+            idstr[vmsd_name_len] != '/') {
              trace_vmstate_subsection_load_bad(vmsd->name, idstr, "(prefix)");
              /* it doesn't have a valid subsection name */
              return 0;
Hi, this breaks migration for s390x and ppc64:

qemu-system-s390x: Missing section footer for qemu-s390-flic

Thanks for reply! It happens because "qemu-s390-flic" vmsd has "qemu-s390-flic-full" subsection. I'm not sure if we can change the names of the existing subsections. Peter, what do you think?

It seemsto be a biggerproblemthatthe migrationframeworkdoesnotdocumentthe namesforsubsections in any way.Atthe same time, the codeimpliesthat atleastthe namesshould bea substringwithitsparent. There is even such a code comment `subsection name has to be "section_name/a"`, that seemsto implythe presenceof /.

Also I tried to put together a list of devices whose separator in subsections is not /:

pflash_cfi01
or-irq
vga
ide bmdma (this one is interesting because using both `_` and `/` as separator
pckbd
apic
arm_gic
qemu-s390-flic
iotkit_secctl
spapr_iommu
spapr
arm.cortex-a9-global-timer
vhost-user-fs

They using `_`, `-` or `.` as separator

Reply via email to