Re: [PATCH] gnttab: avoid triggering assertion in radix_tree_ulong_to_ptr()

[Andrew Cooper <andrew.cooper3@xxxxxxxxxx>]

On 27/08/2021 09:21, Jan Beulich wrote:
> Relevant quotes from the C11 standard:
>
> "Except where explicitly stated otherwise, for the purposes of this
>  subclause unnamed members of objects of structure and union type do not
>  participate in initialization. Unnamed members of structure objects
>  have indeterminate value even after initialization."
>
> "If there are fewer initializers in a brace-enclosed list than there are
>  elements or members of an aggregate, [...], the remainder of the
>  aggregate shall be initialized implicitly the same as objects that have
>  static storage duration."
>
> "If an object that has static or thread storage duration is not
>  initialized explicitly, then:
>  [...]
>  — if it is an aggregate, every member is initialized (recursively)
>    according to these rules, and any padding is initialized to zero
>    bits;
>  [...]"
>
> "A bit-field declaration with no declarator, but only a colon and a
>  width, indicates an unnamed bit-field." Footnote: "An unnamed bit-field
>  structure member is useful for padding to conform to externally imposed
>  layouts."
>
> "There may be unnamed padding within a structure object, but not at its
>  beginning."
>
> Which makes me conclude:
> - Whether an unnamed bit-field member is an unnamed member or padding is
>   unclear, and hence also whether the last quote above would render the
>   big endian case of the structure declaration invalid.
> - Whether the number of members of an aggregate includes unnamed ones is
>   also not really clear.
> - The initializer in map_grant_ref() initializes all fields of the "cnt"
>   sub-structure of the union, so assuming the second quote above applies
>   here (indirectly), the compiler isn't required to implicitly
>   initialize the rest (i.e. in particular any padding) like would happen
>   for static storage duration objects.
>
> Gcc 7.4.1 can be observed (apparently in debug builds only) to translate
> aforementioned initializer to a read-modify-write operation of a stack
> variable, leaving unchanged the top two bits of whatever was previously
> in that stack slot. Clearly if either of the two bits were set,
> radix_tree_ulong_to_ptr()'s assertion would trigger.
>
> Therefore, to be on the safe side, add an explicit padding field for the
> non-big-endian-bitfields case and give a dummy name to both padding
> fields.
>
> Fixes: 9781b51efde2 ("gnttab: replace mapkind()")
> Signed-off-by: Jan Beulich <jbeulich@xxxxxxxx>

Acked-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>