Skip to content

Commit e136a40

Browse files
committed
randstruct: gcc-plugin: Remove bogus void member
When building the randomized replacement tree of struct members, the randstruct GCC plugin would insert, as the first member, a 0-sized void member. This appears as though it was done to catch non-designated ("unnamed") static initializers, which wouldn't be stable since they depend on the original struct layout order. This was accomplished by having the side-effect of the "void member" tripping an assert in GCC internals (count_type_elements) if the member list ever needed to be counted (e.g. for figuring out the order of members during a non-designated initialization), which would catch impossible type (void) in the struct: security/landlock/fs.c: In function ‘hook_file_ioctl_common’: security/landlock/fs.c:1745:61: internal compiler error: in count_type_elements, at expr.cc:7075 1745 | .u.op = &(struct lsm_ioctlop_audit) { | ^ static HOST_WIDE_INT count_type_elements (const_tree type, bool for_ctor_p) { switch (TREE_CODE (type)) ... case VOID_TYPE: default: gcc_unreachable (); } } However this is a redundant safety measure since randstruct uses the __designated_initializer attribute both internally and within the __randomized_layout attribute macro so that this would be enforced by the compiler directly even when randstruct was not enabled (via -Wdesignated-init). A recent change in Landlock ended up tripping the same member counting routine when using a full-struct copy initializer as part of an anonymous initializer. This, however, is a false positive as the initializer is copying between identical structs (and hence identical layouts). The "path" member is "struct path", a randomized struct, and is being copied to from another "struct path", the "f_path" member: landlock_log_denial(landlock_cred(file->f_cred), &(struct landlock_request) { .type = LANDLOCK_REQUEST_FS_ACCESS, .audit = { .type = LSM_AUDIT_DATA_IOCTL_OP, .u.op = &(struct lsm_ioctlop_audit) { .path = file->f_path, .cmd = cmd, }, }, ... As can be seen with the coming randstruct KUnit test, there appears to be no behavioral problems with this kind of initialization when the void member is removed from the randstruct GCC plugin, so remove it. Reported-by: "Dr. David Alan Gilbert" <[email protected]> Closes: https://lore.kernel.org/lkml/Z_PRaKx7q70MKgCA@gallifrey/ Reported-by: Mark Brown <[email protected]> Closes: https://lore.kernel.org/lkml/[email protected]/ Reported-by: WangYuli <[email protected]> Closes: https://lore.kernel.org/lkml/337D5D4887277B27+3c677db3-a8b9-47f0-93a4-7809355f1381@uniontech.com/ Fixes: 313dd1b ("gcc-plugins: Add the randstruct plugin") Signed-off-by: Kees Cook <[email protected]>
1 parent 960013e commit e136a40

File tree

1 file changed

+1
-17
lines changed

1 file changed

+1
-17
lines changed

scripts/gcc-plugins/randomize_layout_plugin.c

Lines changed: 1 addition & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -344,29 +344,13 @@ static int relayout_struct(tree type)
344344

345345
shuffle(type, (tree *)newtree, shuffle_length);
346346

347-
/*
348-
* set up a bogus anonymous struct field designed to error out on unnamed struct initializers
349-
* as gcc provides no other way to detect such code
350-
*/
351-
list = make_node(FIELD_DECL);
352-
TREE_CHAIN(list) = newtree[0];
353-
TREE_TYPE(list) = void_type_node;
354-
DECL_SIZE(list) = bitsize_zero_node;
355-
DECL_NONADDRESSABLE_P(list) = 1;
356-
DECL_FIELD_BIT_OFFSET(list) = bitsize_zero_node;
357-
DECL_SIZE_UNIT(list) = size_zero_node;
358-
DECL_FIELD_OFFSET(list) = size_zero_node;
359-
DECL_CONTEXT(list) = type;
360-
// to satisfy the constify plugin
361-
TREE_READONLY(list) = 1;
362-
363347
for (i = 0; i < num_fields - 1; i++)
364348
TREE_CHAIN(newtree[i]) = newtree[i+1];
365349
TREE_CHAIN(newtree[num_fields - 1]) = NULL_TREE;
366350

367351
main_variant = TYPE_MAIN_VARIANT(type);
368352
for (variant = main_variant; variant; variant = TYPE_NEXT_VARIANT(variant)) {
369-
TYPE_FIELDS(variant) = list;
353+
TYPE_FIELDS(variant) = newtree[0];
370354
TYPE_ATTRIBUTES(variant) = copy_list(TYPE_ATTRIBUTES(variant));
371355
TYPE_ATTRIBUTES(variant) = tree_cons(get_identifier("randomize_performed"), NULL_TREE, TYPE_ATTRIBUTES(variant));
372356
TYPE_ATTRIBUTES(variant) = tree_cons(get_identifier("designated_init"), NULL_TREE, TYPE_ATTRIBUTES(variant));

0 commit comments

Comments
 (0)