Linux Hardening
 help / color / mirror / Atom feed
From: Kees Cook <kees@kernel.org>
To: Arnd Bergmann <arnd@arndb.de>
Cc: "Kees Cook" <kees@kernel.org>,
	"Dr. David Alan Gilbert" <linux@treblig.org>,
	"Mark Brown" <broonie@kernel.org>,
	WangYuli <wangyuli@uniontech.com>,
	"Mickaël Salaün" <mic@digikod.net>,
	"Günther Noack" <gnoack@google.com>,
	"Bill Wendling" <morbo@google.com>,
	"Andrew Morton" <akpm@linux-foundation.org>,
	"Nathan Chancellor" <nathan@kernel.org>,
	"Nick Desaulniers" <nick.desaulniers+lkml@gmail.com>,
	"Justin Stitt" <justinstitt@google.com>,
	"Petr Mladek" <pmladek@suse.com>,
	"David Gow" <davidgow@google.com>, "Rae Moar" <rmoar@google.com>,
	"Tamir Duberstein" <tamird@gmail.com>,
	"Diego Vieira" <diego.daniel.professional@gmail.com>,
	"Luis Chamberlain" <mcgrof@kernel.org>,
	"Gustavo A. R. Silva" <gustavoars@kernel.org>,
	"Paul Moore" <paul@paul-moore.com>,
	"James Morris" <jmorris@namei.org>,
	"Serge E. Hallyn" <serge@hallyn.com>,
	linux-kernel@vger.kernel.org, llvm@lists.linux.dev,
	linux-hardening@vger.kernel.org,
	linux-security-module@vger.kernel.org
Subject: [PATCH 1/3] randstruct: gcc-plugin: Remove bogus void member
Date: Sat, 26 Apr 2025 18:38:33 -0700	[thread overview]
Message-ID: <20250427013836.877214-1-kees@kernel.org> (raw)
In-Reply-To: <20250427013604.work.926-kees@kernel.org>

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" <linux@treblig.org>
Closes: https://lore.kernel.org/lkml/Z_PRaKx7q70MKgCA@gallifrey/
Reported-by: Mark Brown <broonie@kernel.org>
Closes: https://lore.kernel.org/lkml/20250407-kbuild-disable-gcc-plugins-v1-1-5d46ae583f5e@kernel.org/
Reported-by: WangYuli <wangyuli@uniontech.com>
Closes: https://lore.kernel.org/lkml/337D5D4887277B27+3c677db3-a8b9-47f0-93a4-7809355f1381@uniontech.com/
Fixes: 313dd1b62921 ("gcc-plugins: Add the randstruct plugin")
Signed-off-by: Kees Cook <kees@kernel.org>
---
Cc: "Mickaël Salaün" <mic@digikod.net>
Cc: Mark Brown <broonie@kernel.org>
Cc: "Günther Noack" <gnoack@google.com>
Cc: Arnd Bergmann <arnd@arndb.de>
---
 scripts/gcc-plugins/randomize_layout_plugin.c | 18 +-----------------
 1 file changed, 1 insertion(+), 17 deletions(-)

diff --git a/scripts/gcc-plugins/randomize_layout_plugin.c b/scripts/gcc-plugins/randomize_layout_plugin.c
index 5694df3da2e9..971a1908a8cc 100644
--- a/scripts/gcc-plugins/randomize_layout_plugin.c
+++ b/scripts/gcc-plugins/randomize_layout_plugin.c
@@ -344,29 +344,13 @@ static int relayout_struct(tree type)
 
 	shuffle(type, (tree *)newtree, shuffle_length);
 
-	/*
-	 * set up a bogus anonymous struct field designed to error out on unnamed struct initializers
-	 * as gcc provides no other way to detect such code
-	 */
-	list = make_node(FIELD_DECL);
-	TREE_CHAIN(list) = newtree[0];
-	TREE_TYPE(list) = void_type_node;
-	DECL_SIZE(list) = bitsize_zero_node;
-	DECL_NONADDRESSABLE_P(list) = 1;
-	DECL_FIELD_BIT_OFFSET(list) = bitsize_zero_node;
-	DECL_SIZE_UNIT(list) = size_zero_node;
-	DECL_FIELD_OFFSET(list) = size_zero_node;
-	DECL_CONTEXT(list) = type;
-	// to satisfy the constify plugin
-	TREE_READONLY(list) = 1;
-
 	for (i = 0; i < num_fields - 1; i++)
 		TREE_CHAIN(newtree[i]) = newtree[i+1];
 	TREE_CHAIN(newtree[num_fields - 1]) = NULL_TREE;
 
 	main_variant = TYPE_MAIN_VARIANT(type);
 	for (variant = main_variant; variant; variant = TYPE_NEXT_VARIANT(variant)) {
-		TYPE_FIELDS(variant) = list;
+		TYPE_FIELDS(variant) = newtree[0];
 		TYPE_ATTRIBUTES(variant) = copy_list(TYPE_ATTRIBUTES(variant));
 		TYPE_ATTRIBUTES(variant) = tree_cons(get_identifier("randomize_performed"), NULL_TREE, TYPE_ATTRIBUTES(variant));
 		TYPE_ATTRIBUTES(variant) = tree_cons(get_identifier("designated_init"), NULL_TREE, TYPE_ATTRIBUTES(variant));
-- 
2.34.1


  reply	other threads:[~2025-04-27  1:38 UTC|newest]

Thread overview: 17+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-04-27  1:38 [PATCH 0/3] randstruct: gcc-plugin: Remove bogus void member Kees Cook
2025-04-27  1:38 ` Kees Cook [this message]
2025-04-27  1:38 ` [PATCH 2/3] lib/tests: Add randstruct KUnit test Kees Cook
2025-04-27  3:47   ` kernel test robot
2025-04-27  3:47   ` kernel test robot
2025-04-30 18:56     ` Kees Cook
2025-04-27  6:04   ` kernel test robot
2025-04-29  7:44   ` David Gow
2025-04-30 18:56     ` Kees Cook
2025-04-27  1:38 ` [PATCH 3/3] Revert "hardening: Disable GCC randstruct for COMPILE_TEST" Kees Cook
2025-05-30  0:06   ` Thiago Jung Bauermann
2025-05-30  5:12     ` Kees Cook
2025-05-30 19:09       ` Nathan Chancellor
2025-05-30 19:37         ` Kees Cook
2025-05-30 22:31         ` Kees Cook
2025-05-20 15:18 ` [PATCH 0/3] randstruct: gcc-plugin: Remove bogus void member Mickaël Salaün
2025-05-20 16:14   ` Kees Cook

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20250427013836.877214-1-kees@kernel.org \
    --to=kees@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=arnd@arndb.de \
    --cc=broonie@kernel.org \
    --cc=davidgow@google.com \
    --cc=diego.daniel.professional@gmail.com \
    --cc=gnoack@google.com \
    --cc=gustavoars@kernel.org \
    --cc=jmorris@namei.org \
    --cc=justinstitt@google.com \
    --cc=linux-hardening@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-security-module@vger.kernel.org \
    --cc=linux@treblig.org \
    --cc=llvm@lists.linux.dev \
    --cc=mcgrof@kernel.org \
    --cc=mic@digikod.net \
    --cc=morbo@google.com \
    --cc=nathan@kernel.org \
    --cc=nick.desaulniers+lkml@gmail.com \
    --cc=paul@paul-moore.com \
    --cc=pmladek@suse.com \
    --cc=rmoar@google.com \
    --cc=serge@hallyn.com \
    --cc=tamird@gmail.com \
    --cc=wangyuli@uniontech.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox