* [PATCH] kunit/stackinit: Use fill byte different from Clang i386 pattern
@ 2025-03-04 22:56 Kees Cook
2025-03-04 23:13 ` Justin Stitt
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Kees Cook @ 2025-03-04 22:56 UTC (permalink / raw)
To: Nathan Chancellor
Cc: Kees Cook, Bill Wendling, Justin Stitt, llvm, Geert Uytterhoeven,
David Gow, Masami Hiramatsu (Google), Jeff Johnson,
Ard Biesheuvel, linux-kernel, linux-hardening
The byte initialization values used with -ftrivial-auto-var-init=pattern
(CONFIG_INIT_STACK_ALL_PATTERN=y) depends on the compiler, architecture,
and byte position relative to struct member types. On i386 with Clang,
this includes the 0xFF value, which means it looks like nothing changes
between the leaf byte filling pass and the expected "stack wiping"
pass of the stackinit test.
Use the byte fill value of 0x99 instead, fixing the test for i386 Clang
builds.
Reported-by: ernsteiswuerfel
Closes: https://github.com/ClangBuiltLinux/linux/issues/2071
Fixes: 8c30d32b1a32 ("lib/test_stackinit: Handle Clang auto-initialization pattern")
Signed-off-by: Kees Cook <kees@kernel.org>
---
Cc: Nathan Chancellor <nathan@kernel.org>
Cc: Bill Wendling <morbo@google.com>
Cc: Justin Stitt <justinstitt@google.com>
Cc: llvm@lists.linux.dev
---
lib/tests/stackinit_kunit.c | 30 ++++++++++++++++++++----------
1 file changed, 20 insertions(+), 10 deletions(-)
diff --git a/lib/tests/stackinit_kunit.c b/lib/tests/stackinit_kunit.c
index 135322592faf..63aa78e6f5c1 100644
--- a/lib/tests/stackinit_kunit.c
+++ b/lib/tests/stackinit_kunit.c
@@ -184,6 +184,15 @@ static bool stackinit_range_contains(char *haystack_start, size_t haystack_size,
#define INIT_UNION_assigned_copy(var_type) \
INIT_STRUCT_assigned_copy(var_type)
+/*
+ * The "did we actually fill the stack?" check value needs
+ * to be neither 0 nor any of the "pattern" bytes. The
+ * pattern bytes are compiler, architecture, and type based,
+ * so we have to pick a value that never appears for those
+ * combinations. Use 0x99 which is not 0xFF, 0xFE, nor 0xAA.
+ */
+#define FILL_BYTE 0x99
+
/*
* @name: unique string name for the test
* @var_type: type to be tested for zeroing initialization
@@ -206,12 +215,12 @@ static noinline void test_ ## name (struct kunit *test) \
ZERO_CLONE_ ## which(zero); \
/* Clear entire check buffer for 0xFF overlap test. */ \
memset(check_buf, 0x00, sizeof(check_buf)); \
- /* Fill stack with 0xFF. */ \
+ /* Fill stack with FILL_BYTE. */ \
ignored = leaf_ ##name((unsigned long)&ignored, 1, \
FETCH_ARG_ ## which(zero)); \
- /* Verify all bytes overwritten with 0xFF. */ \
+ /* Verify all bytes overwritten with FILL_BYTE. */ \
for (sum = 0, i = 0; i < target_size; i++) \
- sum += (check_buf[i] != 0xFF); \
+ sum += (check_buf[i] != FILL_BYTE); \
/* Clear entire check buffer for later bit tests. */ \
memset(check_buf, 0x00, sizeof(check_buf)); \
/* Extract stack-defined variable contents. */ \
@@ -222,7 +231,8 @@ static noinline void test_ ## name (struct kunit *test) \
* possible between the two leaf function calls. \
*/ \
KUNIT_ASSERT_EQ_MSG(test, sum, 0, \
- "leaf fill was not 0xFF!?\n"); \
+ "leaf fill was not 0x%02X!?\n", \
+ FILL_BYTE); \
\
/* Validate that compiler lined up fill and target. */ \
KUNIT_ASSERT_TRUE_MSG(test, \
@@ -234,9 +244,9 @@ static noinline void test_ ## name (struct kunit *test) \
(int)((ssize_t)(uintptr_t)fill_start - \
(ssize_t)(uintptr_t)target_start)); \
\
- /* Look for any bytes still 0xFF in check region. */ \
+ /* Validate check region has no FILL_BYTE bytes. */ \
for (sum = 0, i = 0; i < target_size; i++) \
- sum += (check_buf[i] == 0xFF); \
+ sum += (check_buf[i] == FILL_BYTE); \
\
if (sum != 0 && xfail) \
kunit_skip(test, \
@@ -271,12 +281,12 @@ static noinline int leaf_ ## name(unsigned long sp, bool fill, \
* stack frame of SOME kind... \
*/ \
memset(buf, (char)(sp & 0xff), sizeof(buf)); \
- /* Fill variable with 0xFF. */ \
+ /* Fill variable with FILL_BYTE. */ \
if (fill) { \
fill_start = &var; \
fill_size = sizeof(var); \
memset(fill_start, \
- (char)((sp & 0xff) | forced_mask), \
+ FILL_BYTE & forced_mask, \
fill_size); \
} \
\
@@ -469,7 +479,7 @@ static int noinline __leaf_switch_none(int path, bool fill)
fill_start = &var;
fill_size = sizeof(var);
- memset(fill_start, forced_mask | 0x55, fill_size);
+ memset(fill_start, (forced_mask | 0x55) & FILL_BYTE, fill_size);
}
memcpy(check_buf, target_start, target_size);
break;
@@ -480,7 +490,7 @@ static int noinline __leaf_switch_none(int path, bool fill)
fill_start = &var;
fill_size = sizeof(var);
- memset(fill_start, forced_mask | 0xaa, fill_size);
+ memset(fill_start, (forced_mask | 0xaa) & FILL_BYTE, fill_size);
}
memcpy(check_buf, target_start, target_size);
break;
--
2.34.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] kunit/stackinit: Use fill byte different from Clang i386 pattern
2025-03-04 22:56 [PATCH] kunit/stackinit: Use fill byte different from Clang i386 pattern Kees Cook
@ 2025-03-04 23:13 ` Justin Stitt
2025-03-04 23:56 ` Kees Cook
2025-03-05 15:12 ` Nathan Chancellor
2025-03-06 6:05 ` Kees Cook
2 siblings, 1 reply; 5+ messages in thread
From: Justin Stitt @ 2025-03-04 23:13 UTC (permalink / raw)
To: Kees Cook
Cc: Nathan Chancellor, Bill Wendling, llvm, Geert Uytterhoeven,
David Gow, Masami Hiramatsu (Google), Jeff Johnson,
Ard Biesheuvel, linux-kernel, linux-hardening
Hi,
On Tue, Mar 04, 2025 at 02:56:11PM -0800, Kees Cook wrote:
> The byte initialization values used with -ftrivial-auto-var-init=pattern
> (CONFIG_INIT_STACK_ALL_PATTERN=y) depends on the compiler, architecture,
> and byte position relative to struct member types. On i386 with Clang,
> this includes the 0xFF value, which means it looks like nothing changes
> between the leaf byte filling pass and the expected "stack wiping"
> pass of the stackinit test.
>
> Use the byte fill value of 0x99 instead, fixing the test for i386 Clang
> builds.
>
> Reported-by: ernsteiswuerfel
> Closes: https://github.com/ClangBuiltLinux/linux/issues/2071
> Fixes: 8c30d32b1a32 ("lib/test_stackinit: Handle Clang auto-initialization pattern")
> Signed-off-by: Kees Cook <kees@kernel.org>
> ---
> Cc: Nathan Chancellor <nathan@kernel.org>
> Cc: Bill Wendling <morbo@google.com>
> Cc: Justin Stitt <justinstitt@google.com>
> Cc: llvm@lists.linux.dev
> ---
> lib/tests/stackinit_kunit.c | 30 ++++++++++++++++++++----------
Ah, couldn't find this file at first. Depends on [1].
> 1 file changed, 20 insertions(+), 10 deletions(-)
>
> diff --git a/lib/tests/stackinit_kunit.c b/lib/tests/stackinit_kunit.c
> index 135322592faf..63aa78e6f5c1 100644
> --- a/lib/tests/stackinit_kunit.c
> +++ b/lib/tests/stackinit_kunit.c
> @@ -184,6 +184,15 @@ static bool stackinit_range_contains(char *haystack_start, size_t haystack_size,
> #define INIT_UNION_assigned_copy(var_type) \
> INIT_STRUCT_assigned_copy(var_type)
>
> +/*
> + * The "did we actually fill the stack?" check value needs
> + * to be neither 0 nor any of the "pattern" bytes. The
> + * pattern bytes are compiler, architecture, and type based,
> + * so we have to pick a value that never appears for those
> + * combinations. Use 0x99 which is not 0xFF, 0xFE, nor 0xAA.
> + */
> +#define FILL_BYTE 0x99
> +
> /*
> * @name: unique string name for the test
> * @var_type: type to be tested for zeroing initialization
> @@ -206,12 +215,12 @@ static noinline void test_ ## name (struct kunit *test) \
> ZERO_CLONE_ ## which(zero); \
> /* Clear entire check buffer for 0xFF overlap test. */ \
> memset(check_buf, 0x00, sizeof(check_buf)); \
> - /* Fill stack with 0xFF. */ \
> + /* Fill stack with FILL_BYTE. */ \
> ignored = leaf_ ##name((unsigned long)&ignored, 1, \
> FETCH_ARG_ ## which(zero)); \
> - /* Verify all bytes overwritten with 0xFF. */ \
> + /* Verify all bytes overwritten with FILL_BYTE. */ \
> for (sum = 0, i = 0; i < target_size; i++) \
> - sum += (check_buf[i] != 0xFF); \
> + sum += (check_buf[i] != FILL_BYTE); \
> /* Clear entire check buffer for later bit tests. */ \
> memset(check_buf, 0x00, sizeof(check_buf)); \
> /* Extract stack-defined variable contents. */ \
> @@ -222,7 +231,8 @@ static noinline void test_ ## name (struct kunit *test) \
> * possible between the two leaf function calls. \
> */ \
> KUNIT_ASSERT_EQ_MSG(test, sum, 0, \
> - "leaf fill was not 0xFF!?\n"); \
> + "leaf fill was not 0x%02X!?\n", \
> + FILL_BYTE); \
KUNIT_ASSERT_EQ_MSG(test, sum, 0, \
- "leaf fill was not 0xFF!?\n"); \
+ "leaf fill was not 0x%02X!??!?!?!?!?!?!?!\n", \
+ FILL_BYTE); \
> \
> /* Validate that compiler lined up fill and target. */ \
> KUNIT_ASSERT_TRUE_MSG(test, \
> @@ -234,9 +244,9 @@ static noinline void test_ ## name (struct kunit *test) \
> (int)((ssize_t)(uintptr_t)fill_start - \
> (ssize_t)(uintptr_t)target_start)); \
> \
> - /* Look for any bytes still 0xFF in check region. */ \
> + /* Validate check region has no FILL_BYTE bytes. */ \
> for (sum = 0, i = 0; i < target_size; i++) \
> - sum += (check_buf[i] == 0xFF); \
> + sum += (check_buf[i] == FILL_BYTE); \
> \
> if (sum != 0 && xfail) \
> kunit_skip(test, \
> @@ -271,12 +281,12 @@ static noinline int leaf_ ## name(unsigned long sp, bool fill, \
> * stack frame of SOME kind... \
> */ \
> memset(buf, (char)(sp & 0xff), sizeof(buf)); \
> - /* Fill variable with 0xFF. */ \
> + /* Fill variable with FILL_BYTE. */ \
> if (fill) { \
> fill_start = &var; \
> fill_size = sizeof(var); \
> memset(fill_start, \
> - (char)((sp & 0xff) | forced_mask), \
> + FILL_BYTE & forced_mask, \
> fill_size); \
> } \
> \
> @@ -469,7 +479,7 @@ static int noinline __leaf_switch_none(int path, bool fill)
> fill_start = &var;
> fill_size = sizeof(var);
>
> - memset(fill_start, forced_mask | 0x55, fill_size);
> + memset(fill_start, (forced_mask | 0x55) & FILL_BYTE, fill_size);
> }
> memcpy(check_buf, target_start, target_size);
> break;
> @@ -480,7 +490,7 @@ static int noinline __leaf_switch_none(int path, bool fill)
> fill_start = &var;
> fill_size = sizeof(var);
>
> - memset(fill_start, forced_mask | 0xaa, fill_size);
> + memset(fill_start, (forced_mask | 0xaa) & FILL_BYTE, fill_size);
> }
> memcpy(check_buf, target_start, target_size);
> break;
> --
> 2.34.1
>
>
I don't understand the stack init pattern tendencies enough to give a RB
but I looked at your patch and it seems to fully replace the fill test
values from 0xff to 0x99, so if 0x99 is OK then OK.
[1]: https://lore.kernel.org/all/20250211003136.2860503-3-kees@kernel.org/
Justin
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] kunit/stackinit: Use fill byte different from Clang i386 pattern
2025-03-04 23:13 ` Justin Stitt
@ 2025-03-04 23:56 ` Kees Cook
0 siblings, 0 replies; 5+ messages in thread
From: Kees Cook @ 2025-03-04 23:56 UTC (permalink / raw)
To: Justin Stitt
Cc: Nathan Chancellor, Bill Wendling, llvm, Geert Uytterhoeven,
David Gow, Masami Hiramatsu (Google), Jeff Johnson,
Ard Biesheuvel, linux-kernel, linux-hardening
On Tue, Mar 04, 2025 at 03:13:20PM -0800, Justin Stitt wrote:
> Hi,
>
> On Tue, Mar 04, 2025 at 02:56:11PM -0800, Kees Cook wrote:
> > The byte initialization values used with -ftrivial-auto-var-init=pattern
> > (CONFIG_INIT_STACK_ALL_PATTERN=y) depends on the compiler, architecture,
> > and byte position relative to struct member types. On i386 with Clang,
> > this includes the 0xFF value, which means it looks like nothing changes
> > between the leaf byte filling pass and the expected "stack wiping"
> > pass of the stackinit test.
> >
> > Use the byte fill value of 0x99 instead, fixing the test for i386 Clang
> > builds.
> >
> > Reported-by: ernsteiswuerfel
> > Closes: https://github.com/ClangBuiltLinux/linux/issues/2071
> > Fixes: 8c30d32b1a32 ("lib/test_stackinit: Handle Clang auto-initialization pattern")
> > Signed-off-by: Kees Cook <kees@kernel.org>
> > ---
> > Cc: Nathan Chancellor <nathan@kernel.org>
> > Cc: Bill Wendling <morbo@google.com>
> > Cc: Justin Stitt <justinstitt@google.com>
> > Cc: llvm@lists.linux.dev
> > ---
> > lib/tests/stackinit_kunit.c | 30 ++++++++++++++++++++----------
>
> Ah, couldn't find this file at first. Depends on [1].
>
> > 1 file changed, 20 insertions(+), 10 deletions(-)
> >
> > diff --git a/lib/tests/stackinit_kunit.c b/lib/tests/stackinit_kunit.c
> > index 135322592faf..63aa78e6f5c1 100644
> > --- a/lib/tests/stackinit_kunit.c
> > +++ b/lib/tests/stackinit_kunit.c
> > @@ -184,6 +184,15 @@ static bool stackinit_range_contains(char *haystack_start, size_t haystack_size,
> > #define INIT_UNION_assigned_copy(var_type) \
> > INIT_STRUCT_assigned_copy(var_type)
> >
> > +/*
> > + * The "did we actually fill the stack?" check value needs
> > + * to be neither 0 nor any of the "pattern" bytes. The
> > + * pattern bytes are compiler, architecture, and type based,
> > + * so we have to pick a value that never appears for those
> > + * combinations. Use 0x99 which is not 0xFF, 0xFE, nor 0xAA.
> > + */
> > +#define FILL_BYTE 0x99
> > +
> > /*
> > * @name: unique string name for the test
> > * @var_type: type to be tested for zeroing initialization
> > @@ -206,12 +215,12 @@ static noinline void test_ ## name (struct kunit *test) \
> > ZERO_CLONE_ ## which(zero); \
> > /* Clear entire check buffer for 0xFF overlap test. */ \
> > memset(check_buf, 0x00, sizeof(check_buf)); \
> > - /* Fill stack with 0xFF. */ \
> > + /* Fill stack with FILL_BYTE. */ \
> > ignored = leaf_ ##name((unsigned long)&ignored, 1, \
> > FETCH_ARG_ ## which(zero)); \
> > - /* Verify all bytes overwritten with 0xFF. */ \
> > + /* Verify all bytes overwritten with FILL_BYTE. */ \
> > for (sum = 0, i = 0; i < target_size; i++) \
> > - sum += (check_buf[i] != 0xFF); \
> > + sum += (check_buf[i] != FILL_BYTE); \
> > /* Clear entire check buffer for later bit tests. */ \
> > memset(check_buf, 0x00, sizeof(check_buf)); \
> > /* Extract stack-defined variable contents. */ \
> > @@ -222,7 +231,8 @@ static noinline void test_ ## name (struct kunit *test) \
> > * possible between the two leaf function calls. \
> > */ \
> > KUNIT_ASSERT_EQ_MSG(test, sum, 0, \
> > - "leaf fill was not 0xFF!?\n"); \
> > + "leaf fill was not 0x%02X!?\n", \
> > + FILL_BYTE); \
>
> KUNIT_ASSERT_EQ_MSG(test, sum, 0, \
> - "leaf fill was not 0xFF!?\n"); \
> + "leaf fill was not 0x%02X!??!?!?!?!?!?!?!\n", \
> + FILL_BYTE); \
OMGWTFBBQ!!
>
> > \
> > /* Validate that compiler lined up fill and target. */ \
> > KUNIT_ASSERT_TRUE_MSG(test, \
> > @@ -234,9 +244,9 @@ static noinline void test_ ## name (struct kunit *test) \
> > (int)((ssize_t)(uintptr_t)fill_start - \
> > (ssize_t)(uintptr_t)target_start)); \
> > \
> > - /* Look for any bytes still 0xFF in check region. */ \
> > + /* Validate check region has no FILL_BYTE bytes. */ \
> > for (sum = 0, i = 0; i < target_size; i++) \
> > - sum += (check_buf[i] == 0xFF); \
> > + sum += (check_buf[i] == FILL_BYTE); \
> > \
> > if (sum != 0 && xfail) \
> > kunit_skip(test, \
> > @@ -271,12 +281,12 @@ static noinline int leaf_ ## name(unsigned long sp, bool fill, \
> > * stack frame of SOME kind... \
> > */ \
> > memset(buf, (char)(sp & 0xff), sizeof(buf)); \
> > - /* Fill variable with 0xFF. */ \
> > + /* Fill variable with FILL_BYTE. */ \
> > if (fill) { \
> > fill_start = &var; \
> > fill_size = sizeof(var); \
> > memset(fill_start, \
> > - (char)((sp & 0xff) | forced_mask), \
> > + FILL_BYTE & forced_mask, \
> > fill_size); \
> > } \
> > \
> > @@ -469,7 +479,7 @@ static int noinline __leaf_switch_none(int path, bool fill)
> > fill_start = &var;
> > fill_size = sizeof(var);
> >
> > - memset(fill_start, forced_mask | 0x55, fill_size);
> > + memset(fill_start, (forced_mask | 0x55) & FILL_BYTE, fill_size);
> > }
> > memcpy(check_buf, target_start, target_size);
> > break;
> > @@ -480,7 +490,7 @@ static int noinline __leaf_switch_none(int path, bool fill)
> > fill_start = &var;
> > fill_size = sizeof(var);
> >
> > - memset(fill_start, forced_mask | 0xaa, fill_size);
> > + memset(fill_start, (forced_mask | 0xaa) & FILL_BYTE, fill_size);
> > }
> > memcpy(check_buf, target_start, target_size);
> > break;
> > --
> > 2.34.1
> >
> >
>
> I don't understand the stack init pattern tendencies enough to give a RB
> but I looked at your patch and it seems to fully replace the fill test
> values from 0xff to 0x99, so if 0x99 is OK then OK.
Yeah, I had to go do some godbolting to remind myself what value were
being used. :)
Thanks for the double-check!
-Kees
>
> [1]: https://lore.kernel.org/all/20250211003136.2860503-3-kees@kernel.org/
>
> Justin
--
Kees Cook
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] kunit/stackinit: Use fill byte different from Clang i386 pattern
2025-03-04 22:56 [PATCH] kunit/stackinit: Use fill byte different from Clang i386 pattern Kees Cook
2025-03-04 23:13 ` Justin Stitt
@ 2025-03-05 15:12 ` Nathan Chancellor
2025-03-06 6:05 ` Kees Cook
2 siblings, 0 replies; 5+ messages in thread
From: Nathan Chancellor @ 2025-03-05 15:12 UTC (permalink / raw)
To: Kees Cook
Cc: Bill Wendling, Justin Stitt, llvm, Geert Uytterhoeven, David Gow,
Masami Hiramatsu (Google), Jeff Johnson, Ard Biesheuvel,
linux-kernel, linux-hardening
On Tue, Mar 04, 2025 at 02:56:11PM -0800, Kees Cook wrote:
> The byte initialization values used with -ftrivial-auto-var-init=pattern
> (CONFIG_INIT_STACK_ALL_PATTERN=y) depends on the compiler, architecture,
> and byte position relative to struct member types. On i386 with Clang,
> this includes the 0xFF value, which means it looks like nothing changes
> between the leaf byte filling pass and the expected "stack wiping"
> pass of the stackinit test.
>
> Use the byte fill value of 0x99 instead, fixing the test for i386 Clang
> builds.
>
> Reported-by: ernsteiswuerfel
> Closes: https://github.com/ClangBuiltLinux/linux/issues/2071
> Fixes: 8c30d32b1a32 ("lib/test_stackinit: Handle Clang auto-initialization pattern")
> Signed-off-by: Kees Cook <kees@kernel.org>
stackinit passes with arm, arm64, i386, and x86_64 when using
LLVM 20.1.0-rc3. Hopefully they do not change the init pattern :)
Tested-by: Nathan Chancellor <nathan@kernel.org>
> ---
> Cc: Nathan Chancellor <nathan@kernel.org>
> Cc: Bill Wendling <morbo@google.com>
> Cc: Justin Stitt <justinstitt@google.com>
> Cc: llvm@lists.linux.dev
> ---
> lib/tests/stackinit_kunit.c | 30 ++++++++++++++++++++----------
> 1 file changed, 20 insertions(+), 10 deletions(-)
>
> diff --git a/lib/tests/stackinit_kunit.c b/lib/tests/stackinit_kunit.c
> index 135322592faf..63aa78e6f5c1 100644
> --- a/lib/tests/stackinit_kunit.c
> +++ b/lib/tests/stackinit_kunit.c
> @@ -184,6 +184,15 @@ static bool stackinit_range_contains(char *haystack_start, size_t haystack_size,
> #define INIT_UNION_assigned_copy(var_type) \
> INIT_STRUCT_assigned_copy(var_type)
>
> +/*
> + * The "did we actually fill the stack?" check value needs
> + * to be neither 0 nor any of the "pattern" bytes. The
> + * pattern bytes are compiler, architecture, and type based,
> + * so we have to pick a value that never appears for those
> + * combinations. Use 0x99 which is not 0xFF, 0xFE, nor 0xAA.
> + */
> +#define FILL_BYTE 0x99
> +
> /*
> * @name: unique string name for the test
> * @var_type: type to be tested for zeroing initialization
> @@ -206,12 +215,12 @@ static noinline void test_ ## name (struct kunit *test) \
> ZERO_CLONE_ ## which(zero); \
> /* Clear entire check buffer for 0xFF overlap test. */ \
> memset(check_buf, 0x00, sizeof(check_buf)); \
> - /* Fill stack with 0xFF. */ \
> + /* Fill stack with FILL_BYTE. */ \
> ignored = leaf_ ##name((unsigned long)&ignored, 1, \
> FETCH_ARG_ ## which(zero)); \
> - /* Verify all bytes overwritten with 0xFF. */ \
> + /* Verify all bytes overwritten with FILL_BYTE. */ \
> for (sum = 0, i = 0; i < target_size; i++) \
> - sum += (check_buf[i] != 0xFF); \
> + sum += (check_buf[i] != FILL_BYTE); \
> /* Clear entire check buffer for later bit tests. */ \
> memset(check_buf, 0x00, sizeof(check_buf)); \
> /* Extract stack-defined variable contents. */ \
> @@ -222,7 +231,8 @@ static noinline void test_ ## name (struct kunit *test) \
> * possible between the two leaf function calls. \
> */ \
> KUNIT_ASSERT_EQ_MSG(test, sum, 0, \
> - "leaf fill was not 0xFF!?\n"); \
> + "leaf fill was not 0x%02X!?\n", \
> + FILL_BYTE); \
> \
> /* Validate that compiler lined up fill and target. */ \
> KUNIT_ASSERT_TRUE_MSG(test, \
> @@ -234,9 +244,9 @@ static noinline void test_ ## name (struct kunit *test) \
> (int)((ssize_t)(uintptr_t)fill_start - \
> (ssize_t)(uintptr_t)target_start)); \
> \
> - /* Look for any bytes still 0xFF in check region. */ \
> + /* Validate check region has no FILL_BYTE bytes. */ \
> for (sum = 0, i = 0; i < target_size; i++) \
> - sum += (check_buf[i] == 0xFF); \
> + sum += (check_buf[i] == FILL_BYTE); \
> \
> if (sum != 0 && xfail) \
> kunit_skip(test, \
> @@ -271,12 +281,12 @@ static noinline int leaf_ ## name(unsigned long sp, bool fill, \
> * stack frame of SOME kind... \
> */ \
> memset(buf, (char)(sp & 0xff), sizeof(buf)); \
> - /* Fill variable with 0xFF. */ \
> + /* Fill variable with FILL_BYTE. */ \
> if (fill) { \
> fill_start = &var; \
> fill_size = sizeof(var); \
> memset(fill_start, \
> - (char)((sp & 0xff) | forced_mask), \
> + FILL_BYTE & forced_mask, \
> fill_size); \
> } \
> \
> @@ -469,7 +479,7 @@ static int noinline __leaf_switch_none(int path, bool fill)
> fill_start = &var;
> fill_size = sizeof(var);
>
> - memset(fill_start, forced_mask | 0x55, fill_size);
> + memset(fill_start, (forced_mask | 0x55) & FILL_BYTE, fill_size);
> }
> memcpy(check_buf, target_start, target_size);
> break;
> @@ -480,7 +490,7 @@ static int noinline __leaf_switch_none(int path, bool fill)
> fill_start = &var;
> fill_size = sizeof(var);
>
> - memset(fill_start, forced_mask | 0xaa, fill_size);
> + memset(fill_start, (forced_mask | 0xaa) & FILL_BYTE, fill_size);
> }
> memcpy(check_buf, target_start, target_size);
> break;
> --
> 2.34.1
>
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] kunit/stackinit: Use fill byte different from Clang i386 pattern
2025-03-04 22:56 [PATCH] kunit/stackinit: Use fill byte different from Clang i386 pattern Kees Cook
2025-03-04 23:13 ` Justin Stitt
2025-03-05 15:12 ` Nathan Chancellor
@ 2025-03-06 6:05 ` Kees Cook
2 siblings, 0 replies; 5+ messages in thread
From: Kees Cook @ 2025-03-06 6:05 UTC (permalink / raw)
To: Nathan Chancellor, Kees Cook
Cc: Bill Wendling, Justin Stitt, llvm, Geert Uytterhoeven, David Gow,
Masami Hiramatsu (Google), Jeff Johnson, Ard Biesheuvel,
linux-kernel, linux-hardening
On Tue, 04 Mar 2025 14:56:11 -0800, Kees Cook wrote:
> The byte initialization values used with -ftrivial-auto-var-init=pattern
> (CONFIG_INIT_STACK_ALL_PATTERN=y) depends on the compiler, architecture,
> and byte position relative to struct member types. On i386 with Clang,
> this includes the 0xFF value, which means it looks like nothing changes
> between the leaf byte filling pass and the expected "stack wiping"
> pass of the stackinit test.
>
> [...]
Applied to for-next/move-kunit-tests.
[1/1] kunit/stackinit: Use fill byte different from Clang i386 pattern
https://git.kernel.org/kees/c/d985e4399adf
--
Kees Cook
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2025-03-06 6:06 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-03-04 22:56 [PATCH] kunit/stackinit: Use fill byte different from Clang i386 pattern Kees Cook
2025-03-04 23:13 ` Justin Stitt
2025-03-04 23:56 ` Kees Cook
2025-03-05 15:12 ` Nathan Chancellor
2025-03-06 6:05 ` Kees Cook
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox