* [PATCH] x86/purgatory: Do not use fortified string functions
@ 2023-05-31 0:33 Kees Cook
2023-05-31 7:51 ` Thorsten Leemhuis
` (2 more replies)
0 siblings, 3 replies; 5+ messages in thread
From: Kees Cook @ 2023-05-31 0:33 UTC (permalink / raw)
To: Thomas Gleixner
Cc: Kees Cook, Thorsten Leemhuis, Joan Bruguera Micó,
Ingo Molnar, Borislav Petkov, Dave Hansen, x86, H. Peter Anvin,
Nick Desaulniers, Masahiro Yamada, Peter Zijlstra (Intel),
Alyssa Ross, Sami Tolvanen, Alexander Potapenko,
Gustavo A. R. Silva, linux-kernel, linux-hardening
With the addition of -fstrict-flex-arrays=3, struct sha256_state's
trailing array is no longer ignored by CONFIG_FORTIFY_SOURCE:
struct sha256_state {
u32 state[SHA256_DIGEST_SIZE / 4];
u64 count;
u8 buf[SHA256_BLOCK_SIZE];
};
This means that the memcpy() calls with "buf" as a destination in
sha256.c's code will attempt to perform run-time bounds checking, which
could lead to calling missing functions, specifically a potential
WARN_ONCE, which isn't callable from purgatory.
Reported-by: Thorsten Leemhuis <linux@leemhuis.info>
Closes: https://lore.kernel.org/lkml/175578ec-9dec-7a9c-8d3a-43f24ff86b92@leemhuis.info/
Bisected-by: "Joan Bruguera Micó" <joanbrugueram@gmail.com>
Fixes: df8fc4e934c1 ("kbuild: Enable -fstrict-flex-arrays=3")
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: Ingo Molnar <mingo@redhat.com>
Cc: Borislav Petkov <bp@alien8.de>
Cc: Dave Hansen <dave.hansen@linux.intel.com>
Cc: x86@kernel.org
Cc: "H. Peter Anvin" <hpa@zytor.com>
Cc: Nick Desaulniers <ndesaulniers@google.com>
Cc: Masahiro Yamada <masahiroy@kernel.org>
Cc: "Peter Zijlstra (Intel)" <peterz@infradead.org>
Cc: Alyssa Ross <hi@alyssa.is>
Cc: Sami Tolvanen <samitolvanen@google.com>
Cc: Alexander Potapenko <glider@google.com>
Signed-off-by: Kees Cook <keescook@chromium.org>
---
arch/x86/purgatory/Makefile | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/arch/x86/purgatory/Makefile b/arch/x86/purgatory/Makefile
index 82fec66d46d2..005324d6c76b 100644
--- a/arch/x86/purgatory/Makefile
+++ b/arch/x86/purgatory/Makefile
@@ -12,7 +12,7 @@ $(obj)/string.o: $(srctree)/arch/x86/boot/compressed/string.c FORCE
$(obj)/sha256.o: $(srctree)/lib/crypto/sha256.c FORCE
$(call if_changed_rule,cc_o_c)
-CFLAGS_sha256.o := -D__DISABLE_EXPORTS
+CFLAGS_sha256.o := -D__DISABLE_EXPORTS -D__NO_FORTIFY
# When linking purgatory.ro with -r unresolved symbols are not checked,
# also link a purgatory.chk binary without -r to check for unresolved symbols.
--
2.34.1
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] x86/purgatory: Do not use fortified string functions
2023-05-31 0:33 [PATCH] x86/purgatory: Do not use fortified string functions Kees Cook
@ 2023-05-31 7:51 ` Thorsten Leemhuis
2023-06-01 16:45 ` Dave Hansen
2023-06-01 16:57 ` Kees Cook
2 siblings, 0 replies; 5+ messages in thread
From: Thorsten Leemhuis @ 2023-05-31 7:51 UTC (permalink / raw)
To: Kees Cook, Thomas Gleixner
Cc: Joan Bruguera Micó,
Ingo Molnar, Borislav Petkov, Dave Hansen, x86, H. Peter Anvin,
Nick Desaulniers, Masahiro Yamada, Peter Zijlstra (Intel),
Alyssa Ross, Sami Tolvanen, Alexander Potapenko,
Gustavo A. R. Silva, linux-kernel, linux-hardening
Kees, thx for looking into this.
Joan, thx for bisecting. I should have done this earlier myself...
On 31.05.23 02:33, Kees Cook wrote:
> With the addition of -fstrict-flex-arrays=3, struct sha256_state's
> trailing array is no longer ignored by CONFIG_FORTIFY_SOURCE:
>
> struct sha256_state {
> u32 state[SHA256_DIGEST_SIZE / 4];
> u64 count;
> u8 buf[SHA256_BLOCK_SIZE];
> };
>
> This means that the memcpy() calls with "buf" as a destination in
> sha256.c's code will attempt to perform run-time bounds checking, which
> could lead to calling missing functions, specifically a potential
> WARN_ONCE, which isn't callable from purgatory.
>
> Reported-by: Thorsten Leemhuis <linux@leemhuis.info>
> Closes: https://lore.kernel.org/lkml/175578ec-9dec-7a9c-8d3a-43f24ff86b92@leemhuis.info/
Did a test build and booted it in a Vm, everything seems fine. So thx
again and feel free to add:
Tested-by: Thorsten Leemhuis <linux@leemhuis.info>
Ciao, Thorsten
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] x86/purgatory: Do not use fortified string functions
2023-05-31 0:33 [PATCH] x86/purgatory: Do not use fortified string functions Kees Cook
2023-05-31 7:51 ` Thorsten Leemhuis
@ 2023-06-01 16:45 ` Dave Hansen
2023-06-01 16:50 ` Kees Cook
2023-06-01 16:57 ` Kees Cook
2 siblings, 1 reply; 5+ messages in thread
From: Dave Hansen @ 2023-06-01 16:45 UTC (permalink / raw)
To: Kees Cook, Thomas Gleixner
Cc: Thorsten Leemhuis, Joan Bruguera Micó,
Ingo Molnar, Borislav Petkov, Dave Hansen, x86, H. Peter Anvin,
Nick Desaulniers, Masahiro Yamada, Peter Zijlstra (Intel),
Alyssa Ross, Sami Tolvanen, Alexander Potapenko,
Gustavo A. R. Silva, linux-kernel, linux-hardening
On 5/30/23 17:33, Kees Cook wrote:
> With the addition of -fstrict-flex-arrays=3, struct sha256_state's
> trailing array is no longer ignored by CONFIG_FORTIFY_SOURCE:
>
> struct sha256_state {
> u32 state[SHA256_DIGEST_SIZE / 4];
> u64 count;
> u8 buf[SHA256_BLOCK_SIZE];
> };
>
> This means that the memcpy() calls with "buf" as a destination in
> sha256.c's code will attempt to perform run-time bounds checking, which
> could lead to calling missing functions, specifically a potential
> WARN_ONCE, which isn't callable from purgatory.
>
> Reported-by: Thorsten Leemhuis <linux@leemhuis.info>
> Closes: https://lore.kernel.org/lkml/175578ec-9dec-7a9c-8d3a-43f24ff86b92@leemhuis.info/
> Bisected-by: "Joan Bruguera Micó" <joanbrugueram@gmail.com>
> Fixes: df8fc4e934c1 ("kbuild: Enable -fstrict-flex-arrays=3")
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: Ingo Molnar <mingo@redhat.com>
Hi Folks,
The -fstrict-flex-arrays=3 commit isn't upstream yet, right? That makes
it a _bit_ wonky for us to carry this on the x86 side since among other
things, the Fixes commit doesn't exist. I'd be fine if this goes up
along with the -fstrict-flex-arrays=3 code, so:
Acked-by: Dave Hansen <dave.hansen@linux.intel.com>
We could also pick it up from the x86 side, but I think that would need
a _bit_ of a different commit message to allude to it being to prepare
for the _future_ setting of -fstrict-flex-arrays=3 and having no
practical benefits now.
Let me know if you don't want to send this up with the
-fstrict-flex-arrays=3 set.
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] x86/purgatory: Do not use fortified string functions
2023-06-01 16:45 ` Dave Hansen
@ 2023-06-01 16:50 ` Kees Cook
0 siblings, 0 replies; 5+ messages in thread
From: Kees Cook @ 2023-06-01 16:50 UTC (permalink / raw)
To: Dave Hansen
Cc: Thomas Gleixner, Thorsten Leemhuis, Joan Bruguera Micó,
Ingo Molnar, Borislav Petkov, Dave Hansen, x86, H. Peter Anvin,
Nick Desaulniers, Masahiro Yamada, Peter Zijlstra (Intel),
Alyssa Ross, Sami Tolvanen, Alexander Potapenko,
Gustavo A. R. Silva, linux-kernel, linux-hardening
On Thu, Jun 01, 2023 at 09:45:57AM -0700, Dave Hansen wrote:
> On 5/30/23 17:33, Kees Cook wrote:
> > With the addition of -fstrict-flex-arrays=3, struct sha256_state's
> > trailing array is no longer ignored by CONFIG_FORTIFY_SOURCE:
> >
> > struct sha256_state {
> > u32 state[SHA256_DIGEST_SIZE / 4];
> > u64 count;
> > u8 buf[SHA256_BLOCK_SIZE];
> > };
> >
> > This means that the memcpy() calls with "buf" as a destination in
> > sha256.c's code will attempt to perform run-time bounds checking, which
> > could lead to calling missing functions, specifically a potential
> > WARN_ONCE, which isn't callable from purgatory.
> >
> > Reported-by: Thorsten Leemhuis <linux@leemhuis.info>
> > Closes: https://lore.kernel.org/lkml/175578ec-9dec-7a9c-8d3a-43f24ff86b92@leemhuis.info/
> > Bisected-by: "Joan Bruguera Micó" <joanbrugueram@gmail.com>
> > Fixes: df8fc4e934c1 ("kbuild: Enable -fstrict-flex-arrays=3")
> > Cc: Thomas Gleixner <tglx@linutronix.de>
> > Cc: Ingo Molnar <mingo@redhat.com>
>
> Hi Folks,
>
> The -fstrict-flex-arrays=3 commit isn't upstream yet, right? That makes
Correct.
> it a _bit_ wonky for us to carry this on the x86 side since among other
> things, the Fixes commit doesn't exist. I'd be fine if this goes up
> along with the -fstrict-flex-arrays=3 code, so:
>
> Acked-by: Dave Hansen <dave.hansen@linux.intel.com>
That would be perfect; thank you! I've added it to my tree.
--
Kees Cook
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] x86/purgatory: Do not use fortified string functions
2023-05-31 0:33 [PATCH] x86/purgatory: Do not use fortified string functions Kees Cook
2023-05-31 7:51 ` Thorsten Leemhuis
2023-06-01 16:45 ` Dave Hansen
@ 2023-06-01 16:57 ` Kees Cook
2 siblings, 0 replies; 5+ messages in thread
From: Kees Cook @ 2023-06-01 16:57 UTC (permalink / raw)
To: tglx, keescook
Cc: masahiroy, mingo, joanbrugueram, peterz, hi, glider, dave.hansen,
linux-kernel, linux-hardening, linux, samitolvanen, hpa,
ndesaulniers, bp, gustavoars, x86
On Tue, 30 May 2023 17:33:48 -0700, Kees Cook wrote:
> With the addition of -fstrict-flex-arrays=3, struct sha256_state's
> trailing array is no longer ignored by CONFIG_FORTIFY_SOURCE:
>
> struct sha256_state {
> u32 state[SHA256_DIGEST_SIZE / 4];
> u64 count;
> u8 buf[SHA256_BLOCK_SIZE];
> };
>
> [...]
Applied to for-next/hardening, thanks!
[1/1] x86/purgatory: Do not use fortified string functions
https://git.kernel.org/kees/c/f6ab7fc96a53
--
Kees Cook
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2023-06-01 16:58 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-05-31 0:33 [PATCH] x86/purgatory: Do not use fortified string functions Kees Cook
2023-05-31 7:51 ` Thorsten Leemhuis
2023-06-01 16:45 ` Dave Hansen
2023-06-01 16:50 ` Kees Cook
2023-06-01 16:57 ` Kees Cook
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).