On 2/23/26 23:58, Peter Xu wrote:
On Fri, Feb 20, 2026 at 03:26:29PM +0500, 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.
Fix it by checking `/` at the end of idstr.
Checking versus '\' looks reasonable, however I'm still confused on the
example given, and what problem you hit.
Here, your concern seems to be that vmstate_subsection_load() can
accidentally load a FIELD of the parent VMSD whose name is exactly the name
of the parent VMSD (which will be the prefix of all subsections).
Thanks for reply,
vmstate_subsection_load() while trying to load a FIELD (whose name is
prefix of parent name) of the parent VMSD will try to load parent
subsection.
Now my question is, when reaching the line you modified below, it needs to
be prefixed with QEMU_VM_SUBSECTION. It means the src QEMU is dumping a
subsection rather than a field. OTOH, when dumping a field, we never dump
any name; I don't think we name FIELD at all..
Could you share the failure you hit in real life? That might help to
understand the problem on its own.
Here is code example:
```
static const VMStateDescription vmstate_vhost_user_virtio_blk_inflight = {
.name = "virtio-blk/inflight",
.version_id = 2,
.needed = vhost_user_blk_inflight_needed,
.fields = (const VMStateField[]) {
VMSTATE_VHOST_INFLIGHT_REGION(inflight, VHostUserBlk),
VMSTATE_END_OF_LIST()
}
};
static const VMStateDescription vmstate_vhost_user_virtio_blk = {
.name = "virtio-blk",
.minimum_version_id = 2,
.version_id = 2,
.fields = (VMStateField[]) {
VMSTATE_VIRTIO_DEVICE,
VMSTATE_END_OF_LIST()
},
.subsections = (const VMStateDescription * []) {
&vmstate_vhost_user_virtio_blk_inflight,
NULL
}
};
```
VMSTATE_VIRTIO_DEVICEĀ is
```
#define VMSTATE_VIRTIO_DEVICE \
{ \
.name = "virtio", \
.info = &virtio_vmstate_info, \
.flags = VMS_SINGLE, \
}
```
So here is "virtio-blk" vmsd that have "virtio" vmsd field and
"virtio-blk/inflight" subsection. This configuration will result in the
fact that inflight vmsd will not be loaded at all (assuming that it met
all the requirements). Qemu logs will contain
trace_vmstate_subsection_load_bad (lookup) error for "virtio" vmstate
when loading "virtio-blk/inflight" subsection.
Thanks
Signed-off-by: Alexandr Moshkov<[email protected]>
---
migration/vmstate.c | 6 ++++--
1 file changed, 4 insertions(+), 2 deletions(-)
diff --git a/migration/vmstate.c b/migration/vmstate.c
index 4d28364f7b..e9901ee349 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,9 @@ 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 ||
+ idstr[vmsd_name_len] != '/') {
trace_vmstate_subsection_load_bad(vmsd->name, idstr, "(prefix)");
/* it doesn't have a valid subsection name */
return 0;
--
2.34.1