Skip to content

Commit

Permalink
kunit/stackinit: Use fill byte different from Clang i386 pattern
Browse files Browse the repository at this point in the history
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: ClangBuiltLinux/linux#2071
Fixes: 8c30d32 ("lib/test_stackinit: Handle Clang auto-initialization pattern")
Tested-by: Nathan Chancellor <[email protected]>
Link: https://lore.kernel.org/r/[email protected]
Signed-off-by: Kees Cook <[email protected]>
  • Loading branch information
kees committed Mar 6, 2025
1 parent 04e403e commit d985e43
Showing 1 changed file with 20 additions and 10 deletions.
30 changes: 20 additions & 10 deletions lib/tests/stackinit_kunit.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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. */ \
Expand All @@ -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, \
Expand All @@ -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, \
Expand Down Expand Up @@ -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); \
} \
\
Expand Down Expand Up @@ -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;
Expand All @@ -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;
Expand Down

0 comments on commit d985e43

Please sign in to comment.