* [PATCH] lib: stackinit: Adjust target string to 8 bytes for m68k
@ 2024-02-27 22:46 Kees Cook
2024-02-27 22:56 ` Andreas Schwab
` (2 more replies)
0 siblings, 3 replies; 4+ messages in thread
From: Kees Cook @ 2024-02-27 22:46 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Kees Cook, Guenter Roeck, linux-kernel, linux-hardening
For reasons I cannot understand, m68k moves the start of the stack frame
for consecutive calls to the same function if the function's test
variable is larger than 8 bytes. This was only happening for the char
array test (obviously), so adjust the length of the string for m68k
only. I want the array size to be longer than "unsigned long" for every
given architecture, so the other remain unchanged.
Additionally adjust the error message to be a bit more clear about
what's happened, and move the KUNIT check outside of the consecutive
calls to minimize what happens between them.
Reported-by: Guenter Roeck <linux@roeck-us.net>
Closes: https://lore.kernel.org/lkml/a0d10d50-2720-4ecd-a2c6-c2c5e5aeee65@roeck-us.net/
Signed-off-by: Kees Cook <keescook@chromium.org>
---
Cc: Geert Uytterhoeven <geert@linux-m68k.org>
---
lib/stackinit_kunit.c | 19 ++++++++++++++++---
1 file changed, 16 insertions(+), 3 deletions(-)
diff --git a/lib/stackinit_kunit.c b/lib/stackinit_kunit.c
index 05947a2feb93..dc3c68f46f0a 100644
--- a/lib/stackinit_kunit.c
+++ b/lib/stackinit_kunit.c
@@ -63,7 +63,16 @@ static bool stackinit_range_contains(char *haystack_start, size_t haystack_size,
#define FETCH_ARG_STRING(var) var
#define FETCH_ARG_STRUCT(var) &var
+/*
+ * On m68k, if the leaf function test variable is longer than 8 bytes,
+ * the start of the stack frame moves. 8 is sufficiently large to
+ * test m68k char arrays, but leave it at 16 for other architectures.
+ */
+#ifdef CONFIG_M68K
+#define FILL_SIZE_STRING 8
+#else
#define FILL_SIZE_STRING 16
+#endif
#define INIT_CLONE_SCALAR /**/
#define INIT_CLONE_STRING [FILL_SIZE_STRING]
@@ -165,19 +174,23 @@ static noinline void test_ ## name (struct kunit *test) \
/* Verify all bytes overwritten with 0xFF. */ \
for (sum = 0, i = 0; i < target_size; i++) \
sum += (check_buf[i] != 0xFF); \
- KUNIT_ASSERT_EQ_MSG(test, sum, 0, \
- "leaf fill was not 0xFF!?\n"); \
/* Clear entire check buffer for later bit tests. */ \
memset(check_buf, 0x00, sizeof(check_buf)); \
/* Extract stack-defined variable contents. */ \
ignored = leaf_ ##name((unsigned long)&ignored, 0, \
FETCH_ARG_ ## which(zero)); \
+ /* \
+ * Delay the sum test to here to do as little as \
+ * possible between the two leaf function calls. \
+ */ \
+ KUNIT_ASSERT_EQ_MSG(test, sum, 0, \
+ "leaf fill was not 0xFF!?\n"); \
\
/* Validate that compiler lined up fill and target. */ \
KUNIT_ASSERT_TRUE_MSG(test, \
stackinit_range_contains(fill_start, fill_size, \
target_start, target_size), \
- "stack fill missed target!? " \
+ "stackframe was not the same between calls!? " \
"(fill %zu wide, target offset by %d)\n", \
fill_size, \
(int)((ssize_t)(uintptr_t)fill_start - \
--
2.34.1
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH] lib: stackinit: Adjust target string to 8 bytes for m68k
2024-02-27 22:46 [PATCH] lib: stackinit: Adjust target string to 8 bytes for m68k Kees Cook
@ 2024-02-27 22:56 ` Andreas Schwab
2024-02-27 23:03 ` Guenter Roeck
2024-02-28 7:58 ` Geert Uytterhoeven
2 siblings, 0 replies; 4+ messages in thread
From: Andreas Schwab @ 2024-02-27 22:56 UTC (permalink / raw)
To: Kees Cook
Cc: Geert Uytterhoeven, Guenter Roeck, linux-kernel, linux-hardening
On Feb 27 2024, Kees Cook wrote:
> For reasons I cannot understand, m68k moves the start of the stack frame
> for consecutive calls to the same function
It's called optimisation.
--
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 7578 EB47 D4E5 4D69 2510 2552 DF73 E780 A9DA AEC1
"And now for something completely different."
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] lib: stackinit: Adjust target string to 8 bytes for m68k
2024-02-27 22:46 [PATCH] lib: stackinit: Adjust target string to 8 bytes for m68k Kees Cook
2024-02-27 22:56 ` Andreas Schwab
@ 2024-02-27 23:03 ` Guenter Roeck
2024-02-28 7:58 ` Geert Uytterhoeven
2 siblings, 0 replies; 4+ messages in thread
From: Guenter Roeck @ 2024-02-27 23:03 UTC (permalink / raw)
To: Kees Cook; +Cc: Geert Uytterhoeven, linux-kernel, linux-hardening
On Tue, Feb 27, 2024 at 02:46:56PM -0800, Kees Cook wrote:
> For reasons I cannot understand, m68k moves the start of the stack frame
> for consecutive calls to the same function if the function's test
> variable is larger than 8 bytes. This was only happening for the char
> array test (obviously), so adjust the length of the string for m68k
> only. I want the array size to be longer than "unsigned long" for every
> given architecture, so the other remain unchanged.
>
> Additionally adjust the error message to be a bit more clear about
> what's happened, and move the KUNIT check outside of the consecutive
> calls to minimize what happens between them.
>
> Reported-by: Guenter Roeck <linux@roeck-us.net>
> Closes: https://lore.kernel.org/lkml/a0d10d50-2720-4ecd-a2c6-c2c5e5aeee65@roeck-us.net/
> Signed-off-by: Kees Cook <keescook@chromium.org>
Hmm, guess I misunderstood the other e-mail.
Anyway, it works. After re-enabling the m68k tests:
Tested-by: Guenter Roeck <linux@roeck-us.net>
I'll also run it through qemu for other architectures to be sure that there
is no negative impact.
Thanks,
Guenter
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: [PATCH] lib: stackinit: Adjust target string to 8 bytes for m68k
2024-02-27 22:46 [PATCH] lib: stackinit: Adjust target string to 8 bytes for m68k Kees Cook
2024-02-27 22:56 ` Andreas Schwab
2024-02-27 23:03 ` Guenter Roeck
@ 2024-02-28 7:58 ` Geert Uytterhoeven
2 siblings, 0 replies; 4+ messages in thread
From: Geert Uytterhoeven @ 2024-02-28 7:58 UTC (permalink / raw)
To: Kees Cook; +Cc: Guenter Roeck, linux-kernel, linux-hardening
Hi Kees,
On Tue, Feb 27, 2024 at 11:47 PM Kees Cook <keescook@chromium.org> wrote:
> For reasons I cannot understand, m68k moves the start of the stack frame
> for consecutive calls to the same function if the function's test
> variable is larger than 8 bytes. This was only happening for the char
> array test (obviously), so adjust the length of the string for m68k
> only. I want the array size to be longer than "unsigned long" for every
> given architecture, so the other remain unchanged.
>
> Additionally adjust the error message to be a bit more clear about
> what's happened, and move the KUNIT check outside of the consecutive
> calls to minimize what happens between them.
Thanks for your patch!
> Reported-by: Guenter Roeck <linux@roeck-us.net>
> Closes: https://lore.kernel.org/lkml/a0d10d50-2720-4ecd-a2c6-c2c5e5aeee65@roeck-us.net/
Do reports have an expiration date? ;-)
Reported-by: Geert Uytterhoeven <geert@linux-m68k.org>
Closes: https://lore.kernel.org/r/CAMuHMdX_g1tbiUL9PUQdqaegrEzCNN3GtbSvSBFYAL4TzvstFg@mail.gmail.com
Closes: https://lore.kernel.org/r/CAMuHMdW6N40+0gGQ+LSrN64Mo4A0-ELAm0pR3gWQ0mNanyBuUQ@mail.gmail.com
> Signed-off-by: Kees Cook <keescook@chromium.org>
Finally all parts of this test are passing on m68k, great!
Tested-by: Geert Uytterhoeven <geert@linux-m68k.org>
Gr{oetje,eeting}s,
Geert
--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org
In personal conversations with technical people, I call myself a hacker. But
when I'm talking to journalists I just say "programmer" or something like that.
-- Linus Torvalds
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2024-02-28 7:58 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2024-02-27 22:46 [PATCH] lib: stackinit: Adjust target string to 8 bytes for m68k Kees Cook
2024-02-27 22:56 ` Andreas Schwab
2024-02-27 23:03 ` Guenter Roeck
2024-02-28 7:58 ` Geert Uytterhoeven
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).