[Qemu-devel] [RFC PATCH v2 7/8] migration/vmstate: fix array of pointers to struct

[Halil Pasic]

Make VMS_ARRAY_OF_POINTER cope with null pointers. Previously the reward
for trying to migrate an array with some null pointers in it was an
illegal memory access, that is a swift and painless death of the process.
Let's make vmstate cope with this scenario at least for pointers to
structs. The general approach is when we encounter a null pointer
(element) instead of following the pointer to save/load the data behind
it we save/load a placeholder. This way we can detect if we expected a
null pointer at the load side but not null data was saved instead. Sadly
all other error scenarios are not detected by this scheme (and would
require the usage of the JSON meta data).

Limitations: Does not work for pointers to primitives.

Signed-off-by: Halil Pasic <address@hidden> Reviewed-by:
Guenther Hutzl <address@hidden> ---

We will need this to load/save some on demand created state from within
the channel subsystem (see ChannelSubSys.css in hw/s390x/css.c for an
example).
---
 include/migration/vmstate.h |  5 +++++
 migration/vmstate.c         | 32 ++++++++++++++++++++++++++++++--
 2 files changed, 35 insertions(+), 2 deletions(-)

diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h
index 5940db0..86d4aca 100644
--- a/include/migration/vmstate.h
+++ b/include/migration/vmstate.h
@@ -236,6 +236,10 @@ extern const VMStateInfo vmstate_info_uint16;
 extern const VMStateInfo vmstate_info_uint32;
 extern const VMStateInfo vmstate_info_uint64;
 
+/** Put this in the stream when migrating a null pointer.*/
+#define VMS_NULLPTR_MARKER ((int8_t) -1)
+extern const VMStateInfo vmstate_info_nullptr;
+
 extern const VMStateInfo vmstate_info_float64;
 extern const VMStateInfo vmstate_info_cpudouble;
 
@@ -453,6 +457,7 @@ extern const VMStateInfo vmstate_info_bitmap;
     .size       = sizeof(_type *),                                    \
     .flags      = VMS_ARRAY|VMS_STRUCT|VMS_ARRAY_OF_POINTER,         \
     .offset     = vmstate_offset_array(_s, _f, _type*, _n),          \
+    .info       = &vmstate_info_nullptr,                              \
 }
 
 #define VMSTATE_STRUCT_SUB_ARRAY(_field, _state, _start, _num, _version, 
_vmsd, _type) { \
diff --git a/migration/vmstate.c b/migration/vmstate.c
index 10a7645..ce3490a 100644
--- a/migration/vmstate.c
+++ b/migration/vmstate.c
@@ -109,7 +109,11 @@ int vmstate_load_state(QEMUFile *f, const 
VMStateDescription *vmsd,
                 if (field->flags & VMS_ARRAY_OF_POINTER) {
                     curr_elem = *(void **)curr_elem;
                 }
-                if (field->flags & VMS_STRUCT) {
+                if (!curr_elem) {
+                    /* if null pointer check placeholder and do not follow */
+                    assert(field->flags & VMS_ARRAY_OF_POINTER);
+                    vmstate_info_nullptr.get(f, curr_elem, size);
+                } else if (field->flags & VMS_STRUCT) {
                     ret = vmstate_load_state(f, field->vmsd, curr_elem,
                                              field->vmsd->version_id);
                 } else {
@@ -320,7 +324,11 @@ void vmstate_save_state(QEMUFile *f, const 
VMStateDescription *vmsd,
                     assert(curr_elem);
                     curr_elem = *(void **)curr_elem;
                 }
-                if (field->flags & VMS_STRUCT) {
+                if (!curr_elem) {
+                    /* if null pointer write placeholder and do not follow */
+                    assert(field->flags & VMS_ARRAY_OF_POINTER);
+                    vmstate_info_nullptr.put(f, curr_elem, size);
+                } else if (field->flags & VMS_STRUCT) {
                     vmstate_save_state(f, field->vmsd, curr_elem, vmdesc_loop);
                 } else {
                     field->info->put(f, curr_elem, size);
@@ -708,6 +716,26 @@ const VMStateInfo vmstate_info_uint64 = {
     .put  = put_uint64,
 };
 
+static int get_nullptr(QEMUFile *f, void *pv, size_t size)
+{
+    int8_t tmp;
+    qemu_get_s8s(f, &tmp);
+    return tmp == VMS_NULLPTR_MARKER ? 0 : -EINVAL;
+}
+
+static void put_nullptr(QEMUFile *f, void *pv, size_t size)
+{
+    int8_t tmp = VMS_NULLPTR_MARKER;
+    assert(pv == NULL);
+    qemu_put_s8s(f, &tmp);
+}
+
+const VMStateInfo vmstate_info_nullptr = {
+    .name = "uint64",
+    .get  = get_nullptr,
+    .put  = put_nullptr,
+};
+
 /* 64 bit unsigned int. See that the received value is the same than the one
    in the field */
 
-- 
2.8.4