Vladimir Sementsov-Ogievskiy <[email protected]> writes:

> On 12.03.26 13:26, Alexandr Moshkov wrote:
>> 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.
>> 
>> To fix this issue, we call vmstate_get_subsection() and, in case of
>> fail, ignore it, pop the VMSD stack and will try to define this
>> subsection for outer VMSD
>> 
>> Signed-off-by: Alexandr Moshkov <[email protected]>
>> ---
>>   migration/vmstate.c | 4 +---
>>   1 file changed, 1 insertion(+), 3 deletions(-)
>> 
>> diff --git a/migration/vmstate.c b/migration/vmstate.c
>> index 4d28364f7b..8f3a69c26e 100644
>> --- a/migration/vmstate.c
>> +++ b/migration/vmstate.c
>> @@ -639,9 +639,7 @@ static int vmstate_subsection_load(QEMUFile *f, const 
>> VMStateDescription *vmsd,
>
> more contex:
>
>          if (strncmp(vmsd->name, idstr, strlen(vmsd->name)) != 0) {
>              trace_vmstate_subsection_load_bad(vmsd->name, idstr, "(prefix)");
>              /* it doesn't have a valid subsection name */
>              return 0;
>          }
>
> If go this way, I think this check for prefix ^^^ should be removed.
>

+1


Reply via email to