* Peter Maydell (peter.mayd...@linaro.org) wrote: > The VMSTATE_STRUCT_VARRAY_UINT32 macro is intended to handle > migrating a field which is an array of structs, but where instead of > migrating the entire array we only migrate a variable number of > elements of it. > > The VMSTATE_STRUCT_VARRAY_POINTER_UINT32 macro is intended to handle > migrating a field which is of pointer type, and points to a > dynamically allocated array of structs of variable size. > > We weren't actually checking that the field passed to > VMSTATE_STRUCT_VARRAY_UINT32 really is an array, with the result that > accidentally using it where the _POINTER_ macro was intended would > compile but silently corrupt memory on migration. > > Add type-checking that enforces that the field passed in is > really of the right array type. This applies to all the VMSTATE > macros which use flags including VMS_VARRAY_* but not VMS_POINTER. > > Signed-off-by: Peter Maydell <peter.mayd...@linaro.org>
> --- > include/migration/vmstate.h | 27 +++++++++++++++++++++------ > 1 file changed, 21 insertions(+), 6 deletions(-) > > diff --git a/include/migration/vmstate.h b/include/migration/vmstate.h > index ca68584eba4..2df333c3612 100644 > --- a/include/migration/vmstate.h > +++ b/include/migration/vmstate.h > @@ -227,8 +227,19 @@ extern const VMStateInfo vmstate_info_bitmap; > extern const VMStateInfo vmstate_info_qtailq; > > #define type_check_2darray(t1,t2,n,m) ((t1(*)[n][m])0 - (t2*)0) > +/* Check that t2 is an array of t1 of size n */ > #define type_check_array(t1,t2,n) ((t1(*)[n])0 - (t2*)0) I'd have to admit I don't understand why that does what you say; I'd expected something to index a t2 pointer with [n]. However, for the rest of it, from migration I'm happy: Reviewed-by: Dr. David Alan Gilbert <dgilb...@redhat.com> given it's just fixing an ARM bug, and given it'll blow up straight away I think it's OK for 4.1; the only risk is if we find a compiler we don't like. > #define type_check_pointer(t1,t2) ((t1**)0 - (t2*)0) > +/* > + * type of element 0 of the specified (array) field of the type. > + * Note that if the field is a pointer then this will return the > + * pointed-to type rather than complaining. > + */ > +#define typeof_elt_of_field(type, field) typeof(((type *)0)->field[0]) > +/* Check that field f in struct type t2 is an array of t1, of any size */ > +#define type_check_varray(t1, t2, f) \ > + (type_check(t1, typeof_elt_of_field(t2, f)) \ > + + QEMU_BUILD_BUG_ON_ZERO(!QEMU_IS_ARRAY(((t2 *)0)->f))) > > #define vmstate_offset_value(_state, _field, _type) \ > (offsetof(_state, _field) + \ > @@ -253,6 +264,10 @@ extern const VMStateInfo vmstate_info_qtailq; > vmstate_offset_array(_state, _field, uint8_t, \ > sizeof(typeof_field(_state, _field))) > > +#define vmstate_offset_varray(_state, _field, _type) \ > + (offsetof(_state, _field) + \ > + type_check_varray(_type, _state, _field)) > + > /* In the macros below, if there is a _version, that means the macro's > * field will be processed only if the version being received is >= > * the _version specified. In general, if you add a new field, you > @@ -347,7 +362,7 @@ extern const VMStateInfo vmstate_info_qtailq; > .info = &(_info), \ > .size = sizeof(_type), \ > .flags = VMS_VARRAY_UINT32|VMS_MULTIPLY_ELEMENTS, \ > - .offset = offsetof(_state, _field), \ > + .offset = vmstate_offset_varray(_state, _field, _type), \ > } > > #define VMSTATE_ARRAY_TEST(_field, _state, _num, _test, _info, _type) {\ > @@ -376,7 +391,7 @@ extern const VMStateInfo vmstate_info_qtailq; > .info = &(_info), \ > .size = sizeof(_type), \ > .flags = VMS_VARRAY_INT32, \ > - .offset = offsetof(_state, _field), \ > + .offset = vmstate_offset_varray(_state, _field, _type), \ > } > > #define VMSTATE_VARRAY_INT32(_field, _state, _field_num, _version, _info, > _type) {\ > @@ -416,7 +431,7 @@ extern const VMStateInfo vmstate_info_qtailq; > .info = &(_info), \ > .size = sizeof(_type), \ > .flags = VMS_VARRAY_UINT16, \ > - .offset = offsetof(_state, _field), \ > + .offset = vmstate_offset_varray(_state, _field, _type), \ > } > > #define VMSTATE_VSTRUCT_TEST(_field, _state, _test, _version, _vmsd, _type, > _struct_version) { \ > @@ -520,7 +535,7 @@ extern const VMStateInfo vmstate_info_qtailq; > .vmsd = &(_vmsd), \ > .size = sizeof(_type), \ > .flags = VMS_STRUCT|VMS_VARRAY_UINT8, \ > - .offset = offsetof(_state, _field), \ > + .offset = vmstate_offset_varray(_state, _field, _type), \ > } > > /* a variable length array (i.e. _type *_field) but we know the > @@ -573,7 +588,7 @@ extern const VMStateInfo vmstate_info_qtailq; > .vmsd = &(_vmsd), \ > .size = sizeof(_type), \ > .flags = VMS_STRUCT|VMS_VARRAY_INT32, \ > - .offset = offsetof(_state, _field), \ > + .offset = vmstate_offset_varray(_state, _field, _type), \ > } > > #define VMSTATE_STRUCT_VARRAY_UINT32(_field, _state, _field_num, _version, > _vmsd, _type) { \ > @@ -583,7 +598,7 @@ extern const VMStateInfo vmstate_info_qtailq; > .vmsd = &(_vmsd), \ > .size = sizeof(_type), \ > .flags = VMS_STRUCT|VMS_VARRAY_UINT32, \ > - .offset = offsetof(_state, _field), \ > + .offset = vmstate_offset_varray(_state, _field, _type), \ > } > > #define VMSTATE_STRUCT_VARRAY_ALLOC(_field, _state, _field_num, _version, > _vmsd, _type) {\ > -- > 2.20.1 > -- Dr. David Alan Gilbert / dgilb...@redhat.com / Manchester, UK