[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

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


  • To: "xen-devel@xxxxxxxxxxxxxxxxxxxx" <xen-devel@xxxxxxxxxxxxxxxxxxxx>
  • From: Jan Beulich <jbeulich@xxxxxxxx>
  • Date: Fri, 27 Aug 2021 10:21:47 +0200
  • Arc-authentication-results: i=1; mx.microsoft.com 1; spf=pass smtp.mailfrom=suse.com; dmarc=pass action=none header.from=suse.com; dkim=pass header.d=suse.com; arc=none
  • Arc-message-signature: i=1; a=rsa-sha256; c=relaxed/relaxed; d=microsoft.com; s=arcselector9901; h=From:Date:Subject:Message-ID:Content-Type:MIME-Version:X-MS-Exchange-SenderADCheck; bh=7vP6cngR9CBrvI0Avsdc6PFiaQqRog3EqvxLzI3sl44=; b=BPqQ7L8/jhfRfvdTrMpvuMJ3IDh3mP4PPHLJQzapcx2WMxI+vPmQXVFx5s0cFvZwlzq+al6Vb91GW5ElezgaYpX91Gop189IMpwiFlZ096l+6N2lD5qi3kCdoS43jcu6r/UGA5REf6Zp3MYZfsIu9q5bOF5zRs864EF/H0mBaTys35zC3NuWOe0BkDe4eB0DeWGtgqDVNFskKNIOgEATviOQJbGj8PYIjoSKQptL7eKJZrnZVK7XjrrZP7GMS8B6WVAZZjg8UTs1e1FeaT01gLebhwWVzOp7DGB4LoR45uVW8/E38JyB/AQ/mxw7gJROSNojLhQCuqHUyhe2QoHYSA==
  • Arc-seal: i=1; a=rsa-sha256; s=arcselector9901; d=microsoft.com; cv=none; b=D4V1sRVkHc0GzAhGO1yhCzpwMdYqQQtZmdU6m99M976bjSET4Q/R0TPpFDCBrCYUFEXCELjW99d8bQAumHPvQIEj3aqYGG3gef0LspqdFZwuWITSSg7WYxLQbkKH7tzaxnQb69lUK4i10tZ2nRx/tbYrhEmDmIcKJ046PPCV00yNFlrjYH55/aIh1dcLHQn63jn6sru8FtnA5TJcXeagQBEkkrh5tobNO8SUD2W4enG4spvxGqN7uGiSgKf0ra1ulZIoddRWjnqHw0pw9xnXHeAALMKkenu7P32sjd0927jM5gRP8air3TGuq8QfEfwSpZJM7oX2a/WFDFfnBGPRgw==
  • Authentication-results: xen.org; dkim=none (message not signed) header.d=none;xen.org; dmarc=none action=none header.from=suse.com;
  • Cc: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>, George Dunlap <george.dunlap@xxxxxxxxxx>, Ian Jackson <iwj@xxxxxxxxxxxxxx>, Julien Grall <julien@xxxxxxx>, Stefano Stabellini <sstabellini@xxxxxxxxxx>, Wei Liu <wl@xxxxxxx>
  • Delivery-date: Fri, 27 Aug 2021 08:22:09 +0000
  • List-id: Xen developer discussion <xen-devel.lists.xenproject.org>

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>

--- a/xen/common/grant_table.c
+++ b/xen/common/grant_table.c
@@ -952,10 +952,13 @@ union maptrack_node {
     struct {
         /* Radix tree slot pointers use two of the bits. */
 #ifdef __BIG_ENDIAN_BITFIELD
-        unsigned long    : 2;
+        unsigned long _0 : 2;
 #endif
         unsigned long rd : BITS_PER_LONG / 2 - 1;
         unsigned long wr : BITS_PER_LONG / 2 - 1;
+#ifndef __BIG_ENDIAN_BITFIELD
+        unsigned long _0 : 2;
+#endif
     } cnt;
     unsigned long raw;
 };




 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.