linux-kernel.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH] x86: remove gcc-x86_*-has-stack-protector.sh checks
@ 2018-11-12  3:06 Masahiro Yamada
  2018-11-12  8:28 ` Kees Cook
  0 siblings, 1 reply; 4+ messages in thread
From: Masahiro Yamada @ 2018-11-12  3:06 UTC (permalink / raw)
  To: Ingo Molnar, Thomas Gleixner, x86
  Cc: Linus Torvalds, Kees Cook, Masahiro Yamada, H. Peter Anvin,
	Borislav Petkov, linux-kernel, Sven Joachim

gcc-x86_64-has-stack-protector.sh was introduced by commit 4f7fd4d7a791
("[PATCH] Add the -fstack-protector option to the CFLAGS") in 2006
to work around buggy compilers.

gcc-x86_32-has-stack-protector.sh was introduced by commit 60a5317ff0f4
("x86: implement x86_32 stack protector"), which did not clearly state
whether compilers were still producing broken code at that time.

Now, the minimum reuquired GCC version is 4.6, which was released in
2011. Probably, we can dump these old compiler checks.

Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
---

 arch/x86/Kconfig                          | 10 +---------
 scripts/gcc-x86_32-has-stack-protector.sh |  4 ----
 scripts/gcc-x86_64-has-stack-protector.sh |  4 ----
 3 files changed, 1 insertion(+), 17 deletions(-)
 delete mode 100755 scripts/gcc-x86_32-has-stack-protector.sh
 delete mode 100755 scripts/gcc-x86_64-has-stack-protector.sh

diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
index 9d734f3..7240d50 100644
--- a/arch/x86/Kconfig
+++ b/arch/x86/Kconfig
@@ -187,7 +187,7 @@ config X86
 	select HAVE_REGS_AND_STACK_ACCESS_API
 	select HAVE_RELIABLE_STACKTRACE		if X86_64 && (UNWINDER_FRAME_POINTER || UNWINDER_ORC) && STACK_VALIDATION
 	select HAVE_FUNCTION_ARG_ACCESS_API
-	select HAVE_STACKPROTECTOR		if CC_HAS_SANE_STACKPROTECTOR
+	select HAVE_STACKPROTECTOR
 	select HAVE_STACK_VALIDATION		if X86_64
 	select HAVE_RSEQ
 	select HAVE_SYSCALL_TRACEPOINTS
@@ -352,14 +352,6 @@ config PGTABLE_LEVELS
 	default 3 if X86_PAE
 	default 2
 
-config CC_HAS_SANE_STACKPROTECTOR
-	bool
-	default $(success,$(srctree)/scripts/gcc-x86_64-has-stack-protector.sh $(CC)) if 64BIT
-	default $(success,$(srctree)/scripts/gcc-x86_32-has-stack-protector.sh $(CC))
-	help
-	   We have to make sure stack protector is unconditionally disabled if
-	   the compiler produces broken code.
-
 menu "Processor type and features"
 
 config ZONE_DMA
diff --git a/scripts/gcc-x86_32-has-stack-protector.sh b/scripts/gcc-x86_32-has-stack-protector.sh
deleted file mode 100755
index f5c1194..0000000
--- a/scripts/gcc-x86_32-has-stack-protector.sh
+++ /dev/null
@@ -1,4 +0,0 @@
-#!/bin/sh
-# SPDX-License-Identifier: GPL-2.0
-
-echo "int foo(void) { char X[200]; return 3; }" | $* -S -x c -c -m32 -O0 -fstack-protector - -o - 2> /dev/null | grep -q "%gs"
diff --git a/scripts/gcc-x86_64-has-stack-protector.sh b/scripts/gcc-x86_64-has-stack-protector.sh
deleted file mode 100755
index 75e4e22..0000000
--- a/scripts/gcc-x86_64-has-stack-protector.sh
+++ /dev/null
@@ -1,4 +0,0 @@
-#!/bin/sh
-# SPDX-License-Identifier: GPL-2.0
-
-echo "int foo(void) { char X[200]; return 3; }" | $* -S -x c -c -m64 -O0 -mcmodel=kernel -fno-PIE -fstack-protector - -o - 2> /dev/null | grep -q "%gs"
-- 
2.7.4


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] x86: remove gcc-x86_*-has-stack-protector.sh checks
  2018-11-12  3:06 [PATCH] x86: remove gcc-x86_*-has-stack-protector.sh checks Masahiro Yamada
@ 2018-11-12  8:28 ` Kees Cook
  2018-11-12 13:54   ` Masahiro Yamada
  0 siblings, 1 reply; 4+ messages in thread
From: Kees Cook @ 2018-11-12  8:28 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: Ingo Molnar, Thomas Gleixner, X86 ML, Linus Torvalds,
	H. Peter Anvin, Borislav Petkov, LKML, Sven Joachim

On Sun, Nov 11, 2018 at 9:06 PM, Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
> gcc-x86_64-has-stack-protector.sh was introduced by commit 4f7fd4d7a791
> ("[PATCH] Add the -fstack-protector option to the CFLAGS") in 2006
> to work around buggy compilers.
>
> gcc-x86_32-has-stack-protector.sh was introduced by commit 60a5317ff0f4
> ("x86: implement x86_32 stack protector"), which did not clearly state
> whether compilers were still producing broken code at that time.
>
> Now, the minimum reuquired GCC version is 4.6, which was released in
> 2011. Probably, we can dump these old compiler checks.

NAK. We need to keep this because we've seen recent regressions with
stack protection (e.g. gcc briefly used global instead of tls for the
canary, which silently broke the use of stack protectors). Since the
gcc/kernel "API" for the canary is so fragile we need to keep these
tests to make sure things end up where they're expected.

-Kees

>
> Signed-off-by: Masahiro Yamada <yamada.masahiro@socionext.com>
> ---
>
>  arch/x86/Kconfig                          | 10 +---------
>  scripts/gcc-x86_32-has-stack-protector.sh |  4 ----
>  scripts/gcc-x86_64-has-stack-protector.sh |  4 ----
>  3 files changed, 1 insertion(+), 17 deletions(-)
>  delete mode 100755 scripts/gcc-x86_32-has-stack-protector.sh
>  delete mode 100755 scripts/gcc-x86_64-has-stack-protector.sh
>
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index 9d734f3..7240d50 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -187,7 +187,7 @@ config X86
>         select HAVE_REGS_AND_STACK_ACCESS_API
>         select HAVE_RELIABLE_STACKTRACE         if X86_64 && (UNWINDER_FRAME_POINTER || UNWINDER_ORC) && STACK_VALIDATION
>         select HAVE_FUNCTION_ARG_ACCESS_API
> -       select HAVE_STACKPROTECTOR              if CC_HAS_SANE_STACKPROTECTOR
> +       select HAVE_STACKPROTECTOR
>         select HAVE_STACK_VALIDATION            if X86_64
>         select HAVE_RSEQ
>         select HAVE_SYSCALL_TRACEPOINTS
> @@ -352,14 +352,6 @@ config PGTABLE_LEVELS
>         default 3 if X86_PAE
>         default 2
>
> -config CC_HAS_SANE_STACKPROTECTOR
> -       bool
> -       default $(success,$(srctree)/scripts/gcc-x86_64-has-stack-protector.sh $(CC)) if 64BIT
> -       default $(success,$(srctree)/scripts/gcc-x86_32-has-stack-protector.sh $(CC))
> -       help
> -          We have to make sure stack protector is unconditionally disabled if
> -          the compiler produces broken code.
> -
>  menu "Processor type and features"
>
>  config ZONE_DMA
> diff --git a/scripts/gcc-x86_32-has-stack-protector.sh b/scripts/gcc-x86_32-has-stack-protector.sh
> deleted file mode 100755
> index f5c1194..0000000
> --- a/scripts/gcc-x86_32-has-stack-protector.sh
> +++ /dev/null
> @@ -1,4 +0,0 @@
> -#!/bin/sh
> -# SPDX-License-Identifier: GPL-2.0
> -
> -echo "int foo(void) { char X[200]; return 3; }" | $* -S -x c -c -m32 -O0 -fstack-protector - -o - 2> /dev/null | grep -q "%gs"
> diff --git a/scripts/gcc-x86_64-has-stack-protector.sh b/scripts/gcc-x86_64-has-stack-protector.sh
> deleted file mode 100755
> index 75e4e22..0000000
> --- a/scripts/gcc-x86_64-has-stack-protector.sh
> +++ /dev/null
> @@ -1,4 +0,0 @@
> -#!/bin/sh
> -# SPDX-License-Identifier: GPL-2.0
> -
> -echo "int foo(void) { char X[200]; return 3; }" | $* -S -x c -c -m64 -O0 -mcmodel=kernel -fno-PIE -fstack-protector - -o - 2> /dev/null | grep -q "%gs"
> --
> 2.7.4
>



-- 
Kees Cook

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] x86: remove gcc-x86_*-has-stack-protector.sh checks
  2018-11-12  8:28 ` Kees Cook
@ 2018-11-12 13:54   ` Masahiro Yamada
  2018-11-12 17:02     ` Kees Cook
  0 siblings, 1 reply; 4+ messages in thread
From: Masahiro Yamada @ 2018-11-12 13:54 UTC (permalink / raw)
  To: Kees Cook
  Cc: Ingo Molnar, Thomas Gleixner, X86 ML, Linus Torvalds,
	H. Peter Anvin, Borislav Petkov, Linux Kernel Mailing List,
	Sven Joachim

On Mon, Nov 12, 2018 at 5:29 PM Kees Cook <keescook@chromium.org> wrote:
>
> On Sun, Nov 11, 2018 at 9:06 PM, Masahiro Yamada
> <yamada.masahiro@socionext.com> wrote:
> > gcc-x86_64-has-stack-protector.sh was introduced by commit 4f7fd4d7a791
> > ("[PATCH] Add the -fstack-protector option to the CFLAGS") in 2006
> > to work around buggy compilers.
> >
> > gcc-x86_32-has-stack-protector.sh was introduced by commit 60a5317ff0f4
> > ("x86: implement x86_32 stack protector"), which did not clearly state
> > whether compilers were still producing broken code at that time.
> >
> > Now, the minimum reuquired GCC version is 4.6, which was released in
> > 2011. Probably, we can dump these old compiler checks.
>
> NAK. We need to keep this because we've seen recent regressions with
> stack protection (e.g. gcc briefly used global instead of tls for the
> canary, which silently broke the use of stack protectors). Since the
> gcc/kernel "API" for the canary is so fragile we need to keep these
> tests to make sure things end up where they're expected.

Thanks for your feedback.

I did not know this is still fragile even after ten years time.

One more curious thing is, x86 is the only arch ever
that has had this kind of script check.



-- 
Best Regards
Masahiro Yamada

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] x86: remove gcc-x86_*-has-stack-protector.sh checks
  2018-11-12 13:54   ` Masahiro Yamada
@ 2018-11-12 17:02     ` Kees Cook
  0 siblings, 0 replies; 4+ messages in thread
From: Kees Cook @ 2018-11-12 17:02 UTC (permalink / raw)
  To: Masahiro Yamada
  Cc: Ingo Molnar, Thomas Gleixner, X86 ML, Linus Torvalds,
	H. Peter Anvin, Borislav Petkov, Linux Kernel Mailing List,
	Sven Joachim

On Mon, Nov 12, 2018 at 7:54 AM, Masahiro Yamada
<yamada.masahiro@socionext.com> wrote:
> On Mon, Nov 12, 2018 at 5:29 PM Kees Cook <keescook@chromium.org> wrote:
>>
>> On Sun, Nov 11, 2018 at 9:06 PM, Masahiro Yamada
>> <yamada.masahiro@socionext.com> wrote:
>> > gcc-x86_64-has-stack-protector.sh was introduced by commit 4f7fd4d7a791
>> > ("[PATCH] Add the -fstack-protector option to the CFLAGS") in 2006
>> > to work around buggy compilers.
>> >
>> > gcc-x86_32-has-stack-protector.sh was introduced by commit 60a5317ff0f4
>> > ("x86: implement x86_32 stack protector"), which did not clearly state
>> > whether compilers were still producing broken code at that time.
>> >
>> > Now, the minimum reuquired GCC version is 4.6, which was released in
>> > 2011. Probably, we can dump these old compiler checks.
>>
>> NAK. We need to keep this because we've seen recent regressions with
>> stack protection (e.g. gcc briefly used global instead of tls for the
>> canary, which silently broke the use of stack protectors). Since the
>> gcc/kernel "API" for the canary is so fragile we need to keep these
>> tests to make sure things end up where they're expected.
>
> Thanks for your feedback.
>
> I did not know this is still fragile even after ten years time.
>
> One more curious thing is, x86 is the only arch ever
> that has had this kind of script check.

Presently, yes -- x86 is the only arch with non-global canaries,
though something may be coming soon for arm64. However, that case may
be more detectable with cc-option. The trouble with gcc was that the
default switched at one point. :(


-- 
Kees Cook

^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2018-11-12 17:02 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-12  3:06 [PATCH] x86: remove gcc-x86_*-has-stack-protector.sh checks Masahiro Yamada
2018-11-12  8:28 ` Kees Cook
2018-11-12 13:54   ` Masahiro Yamada
2018-11-12 17:02     ` 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).