Linux Hardening
 help / color / mirror / Atom feed
From: Kees Cook <kees@kernel.org>
To: Masahiro Yamada <masahiroy@kernel.org>
Cc: Kees Cook <kees@kernel.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Jakub Jelinek <jakub@redhat.com>,
	Nathan Chancellor <nathan@kernel.org>,
	Nicolas Schier <nicolas@fjasle.eu>,
	Nick Desaulniers <ndesaulniers@google.com>,
	Bill Wendling <morbo@google.com>,
	Justin Stitt <justinstitt@google.com>,
	linux-kernel@vger.kernel.org, linux-kbuild@vger.kernel.org,
	llvm@lists.linux.dev, linux-hardening@vger.kernel.org
Subject: [PATCH v2 2/3] stackinit: Add union initialization to selftests
Date: Mon, 27 Jan 2025 11:10:27 -0800	[thread overview]
Message-ID: <20250127191031.245214-2-kees@kernel.org> (raw)
In-Reply-To: <20250127190636.it.745-kees@kernel.org>

The stack initialization selftests were checking scalars, strings,
and structs, but not unions. Add union tests (which are mostly identical
setup to structs). This catches the recent union initialization behavioral
changes seen in GCC 15. Before GCC 15, this new test passes:

    ok 18 test_small_start_old_zero

With GCC 15, it fails:

    not ok 18 test_small_start_old_zero

Specifically, a union with a larger member where a smaller member is
initialized with the older "= { 0 }" syntax:

union test_small_start {
     char one:1;
     char two;
     short three;
     unsigned long four;
     struct big_struct {
             unsigned long array[8];
     } big;
};

This is a regression in compiler behavior that Linux has depended on.
GCC does not seem likely to fix it, instead suggesting that affected
projects start using -fzero-init-padding-bits=unions:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=118403

Signed-off-by: Kees Cook <kees@kernel.org>
---
 lib/stackinit_kunit.c | 103 ++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 103 insertions(+)

diff --git a/lib/stackinit_kunit.c b/lib/stackinit_kunit.c
index 7cc9af181e89..fbe910c9c825 100644
--- a/lib/stackinit_kunit.c
+++ b/lib/stackinit_kunit.c
@@ -47,10 +47,12 @@ static bool stackinit_range_contains(char *haystack_start, size_t haystack_size,
 #define DO_NOTHING_TYPE_SCALAR(var_type)	var_type
 #define DO_NOTHING_TYPE_STRING(var_type)	void
 #define DO_NOTHING_TYPE_STRUCT(var_type)	void
+#define DO_NOTHING_TYPE_UNION(var_type)		void
 
 #define DO_NOTHING_RETURN_SCALAR(ptr)		*(ptr)
 #define DO_NOTHING_RETURN_STRING(ptr)		/**/
 #define DO_NOTHING_RETURN_STRUCT(ptr)		/**/
+#define DO_NOTHING_RETURN_UNION(ptr)		/**/
 
 #define DO_NOTHING_CALL_SCALAR(var, name)			\
 		(var) = do_nothing_ ## name(&(var))
@@ -58,10 +60,13 @@ static bool stackinit_range_contains(char *haystack_start, size_t haystack_size,
 		do_nothing_ ## name(var)
 #define DO_NOTHING_CALL_STRUCT(var, name)			\
 		do_nothing_ ## name(&(var))
+#define DO_NOTHING_CALL_UNION(var, name)			\
+		do_nothing_ ## name(&(var))
 
 #define FETCH_ARG_SCALAR(var)		&var
 #define FETCH_ARG_STRING(var)		var
 #define FETCH_ARG_STRUCT(var)		&var
+#define FETCH_ARG_UNION(var)		&var
 
 /*
  * On m68k, if the leaf function test variable is longer than 8 bytes,
@@ -77,6 +82,7 @@ static bool stackinit_range_contains(char *haystack_start, size_t haystack_size,
 #define INIT_CLONE_SCALAR		/**/
 #define INIT_CLONE_STRING		[FILL_SIZE_STRING]
 #define INIT_CLONE_STRUCT		/**/
+#define INIT_CLONE_UNION		/**/
 
 #define ZERO_CLONE_SCALAR(zero)		memset(&(zero), 0x00, sizeof(zero))
 #define ZERO_CLONE_STRING(zero)		memset(&(zero), 0x00, sizeof(zero))
@@ -92,6 +98,7 @@ static bool stackinit_range_contains(char *haystack_start, size_t haystack_size,
 		zero.three = 0;				\
 		zero.four = 0;				\
 	} while (0)
+#define ZERO_CLONE_UNION(zero)		ZERO_CLONE_STRUCT(zero)
 
 #define INIT_SCALAR_none(var_type)	/**/
 #define INIT_SCALAR_zero(var_type)	= 0
@@ -147,6 +154,34 @@ static bool stackinit_range_contains(char *haystack_start, size_t haystack_size,
 #define INIT_STRUCT_assigned_copy(var_type)				\
 					; var = *(arg)
 
+/* Union initialization is the same as structs. */
+#define INIT_UNION_none(var_type)	INIT_STRUCT_none(var_type)
+#define INIT_UNION_zero(var_type)	INIT_STRUCT_zero(var_type)
+#define INIT_UNION_old_zero(var_type)	INIT_STRUCT_old_zero(var_type)
+
+#define INIT_UNION_static_partial(var_type)		\
+	INIT_STRUCT_static_partial(var_type)
+#define INIT_UNION_static_all(var_type)			\
+	INIT_STRUCT_static_all(var_type)
+#define INIT_UNION_dynamic_partial(var_type)		\
+	INIT_STRUCT_dynamic_partial(var_type)
+#define INIT_UNION_dynamic_all(var_type)		\
+	INIT_STRUCT_dynamic_all(var_type)
+#define INIT_UNION_runtime_partial(var_type)		\
+	INIT_STRUCT_runtime_partial(var_type)
+#define INIT_UNION_runtime_all(var_type)		\
+	INIT_STRUCT_runtime_all(var_type)
+#define INIT_UNION_assigned_static_partial(var_type)	\
+	INIT_STRUCT_assigned_static_partial(var_type)
+#define INIT_UNION_assigned_static_all(var_type)	\
+	INIT_STRUCT_assigned_static_all(var_type)
+#define INIT_UNION_assigned_dynamic_partial(var_type)	\
+	INIT_STRUCT_assigned_dynamic_partial(var_type)
+#define INIT_UNION_assigned_dynamic_all(var_type)	\
+	INIT_STRUCT_assigned_dynamic_all(var_type)
+#define INIT_UNION_assigned_copy(var_type)		\
+	INIT_STRUCT_assigned_copy(var_type)
+
 /*
  * @name: unique string name for the test
  * @var_type: type to be tested for zeroing initialization
@@ -295,6 +330,33 @@ struct test_user {
 	unsigned long four;
 };
 
+/* No padding: all members are the same size. */
+union test_same_sizes {
+	unsigned long one;
+	unsigned long two;
+	unsigned long three;
+	unsigned long four;
+};
+
+/* Mismatched sizes, with one and two being small */
+union test_small_start {
+	char one:1;
+	char two;
+	short three;
+	unsigned long four;
+	struct big_struct {
+		unsigned long array[8];
+	} big;
+};
+
+/* Mismatched sizes, with one and two being small */
+union test_small_end {
+	short one;
+	unsigned long two;
+	char three:1;
+	char four;
+};
+
 #define ALWAYS_PASS	WANT_SUCCESS
 #define ALWAYS_FAIL	XFAIL
 
@@ -333,6 +395,11 @@ struct test_user {
 			    struct test_ ## name, STRUCT, init, \
 			    xfail)
 
+#define DEFINE_UNION_TEST(name, init, xfail)			\
+		DEFINE_TEST(name ## _ ## init,			\
+			    union test_ ## name, STRUCT, init,	\
+			    xfail)
+
 #define DEFINE_STRUCT_TESTS(init, xfail)			\
 		DEFINE_STRUCT_TEST(small_hole, init, xfail);	\
 		DEFINE_STRUCT_TEST(big_hole, init, xfail);	\
@@ -344,10 +411,22 @@ struct test_user {
 				    xfail);			\
 		DEFINE_STRUCT_TESTS(base ## _ ## all, xfail)
 
+#define DEFINE_UNION_INITIALIZER_TESTS(base, xfail)		\
+		DEFINE_UNION_TESTS(base ## _ ## partial,	\
+				    xfail);			\
+		DEFINE_UNION_TESTS(base ## _ ## all, xfail)
+
+#define DEFINE_UNION_TESTS(init, xfail)				\
+		DEFINE_UNION_TEST(same_sizes, init, xfail);	\
+		DEFINE_UNION_TEST(small_start, init, xfail);	\
+		DEFINE_UNION_TEST(small_end, init, xfail);
+
 /* These should be fully initialized all the time! */
 DEFINE_SCALAR_TESTS(zero, ALWAYS_PASS);
 DEFINE_STRUCT_TESTS(zero, ALWAYS_PASS);
 DEFINE_STRUCT_TESTS(old_zero, ALWAYS_PASS);
+DEFINE_UNION_TESTS(zero, ALWAYS_PASS);
+DEFINE_UNION_TESTS(old_zero, ALWAYS_PASS);
 /* Struct initializers: padding may be left uninitialized. */
 DEFINE_STRUCT_INITIALIZER_TESTS(static, STRONG_PASS);
 DEFINE_STRUCT_INITIALIZER_TESTS(dynamic, STRONG_PASS);
@@ -355,6 +434,12 @@ DEFINE_STRUCT_INITIALIZER_TESTS(runtime, STRONG_PASS);
 DEFINE_STRUCT_INITIALIZER_TESTS(assigned_static, STRONG_PASS);
 DEFINE_STRUCT_INITIALIZER_TESTS(assigned_dynamic, STRONG_PASS);
 DEFINE_STRUCT_TESTS(assigned_copy, ALWAYS_FAIL);
+DEFINE_UNION_INITIALIZER_TESTS(static, STRONG_PASS);
+DEFINE_UNION_INITIALIZER_TESTS(dynamic, STRONG_PASS);
+DEFINE_UNION_INITIALIZER_TESTS(runtime, STRONG_PASS);
+DEFINE_UNION_INITIALIZER_TESTS(assigned_static, STRONG_PASS);
+DEFINE_UNION_INITIALIZER_TESTS(assigned_dynamic, STRONG_PASS);
+DEFINE_UNION_TESTS(assigned_copy, ALWAYS_FAIL);
 /* No initialization without compiler instrumentation. */
 DEFINE_SCALAR_TESTS(none, STRONG_PASS);
 DEFINE_STRUCT_TESTS(none, BYREF_PASS);
@@ -438,14 +523,23 @@ DEFINE_TEST_DRIVER(switch_2_none, uint64_t, SCALAR, ALWAYS_FAIL);
 		KUNIT_CASE(test_trailing_hole_ ## init),\
 		KUNIT_CASE(test_packed_ ## init)	\
 
+#define KUNIT_test_unions(init)				\
+		KUNIT_CASE(test_same_sizes_ ## init),	\
+		KUNIT_CASE(test_small_start_ ## init),	\
+		KUNIT_CASE(test_small_end_ ## init)	\
+
 static struct kunit_case stackinit_test_cases[] = {
 	/* These are explicitly initialized and should always pass. */
 	KUNIT_test_scalars(zero),
 	KUNIT_test_structs(zero),
 	KUNIT_test_structs(old_zero),
+	KUNIT_test_unions(zero),
+	KUNIT_test_unions(old_zero),
 	/* Padding here appears to be accidentally always initialized? */
 	KUNIT_test_structs(dynamic_partial),
 	KUNIT_test_structs(assigned_dynamic_partial),
+	KUNIT_test_unions(dynamic_partial),
+	KUNIT_test_unions(assigned_dynamic_partial),
 	/* Padding initialization depends on compiler behaviors. */
 	KUNIT_test_structs(static_partial),
 	KUNIT_test_structs(static_all),
@@ -455,8 +549,17 @@ static struct kunit_case stackinit_test_cases[] = {
 	KUNIT_test_structs(assigned_static_partial),
 	KUNIT_test_structs(assigned_static_all),
 	KUNIT_test_structs(assigned_dynamic_all),
+	KUNIT_test_unions(static_partial),
+	KUNIT_test_unions(static_all),
+	KUNIT_test_unions(dynamic_all),
+	KUNIT_test_unions(runtime_partial),
+	KUNIT_test_unions(runtime_all),
+	KUNIT_test_unions(assigned_static_partial),
+	KUNIT_test_unions(assigned_static_all),
+	KUNIT_test_unions(assigned_dynamic_all),
 	/* Everything fails this since it effectively performs a memcpy(). */
 	KUNIT_test_structs(assigned_copy),
+	KUNIT_test_unions(assigned_copy),
 	/* STRUCTLEAK_BYREF_ALL should cover everything from here down. */
 	KUNIT_test_scalars(none),
 	KUNIT_CASE(test_switch_1_none),
-- 
2.34.1


  parent reply	other threads:[~2025-01-27 19:10 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-01-27 19:10 [PATCH v2 0/3] kbuild: Use -fzero-init-padding-bits=all Kees Cook
2025-01-27 19:10 ` [PATCH v2 1/3] stackinit: Add old-style zero-init syntax to struct tests Kees Cook
2025-01-27 19:10 ` Kees Cook [this message]
2025-01-28  7:46   ` [PATCH v2 2/3] stackinit: Add union initialization to selftests Geert Uytterhoeven
2025-02-03 14:44   ` Geert Uytterhoeven
2025-02-04 15:42     ` Kees Cook
2025-01-27 19:10 ` [PATCH v2 3/3] kbuild: Use -fzero-init-padding-bits=all Kees Cook
2025-01-27 19:54   ` Kees Cook
2025-01-30  2:35   ` Masahiro Yamada

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=20250127191031.245214-2-kees@kernel.org \
    --to=kees@kernel.org \
    --cc=akpm@linux-foundation.org \
    --cc=jakub@redhat.com \
    --cc=justinstitt@google.com \
    --cc=linux-hardening@vger.kernel.org \
    --cc=linux-kbuild@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=llvm@lists.linux.dev \
    --cc=masahiroy@kernel.org \
    --cc=morbo@google.com \
    --cc=nathan@kernel.org \
    --cc=ndesaulniers@google.com \
    --cc=nicolas@fjasle.eu \
    /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