From: Masahiro Yamada <masahiroy@kernel.org>
To: Nick Desaulniers <ndesaulniers@google.com>
Cc: Linux Kbuild mailing list <linux-kbuild@vger.kernel.org>,
clang-built-linux <clang-built-linux@googlegroups.com>,
"maintainer:X86 ARCHITECTURE (32-BIT AND 64-BIT)"
<x86@kernel.org>, linux-efi <linux-efi@vger.kernel.org>,
Kees Cook <keescook@chromium.org>,
LKML <linux-kernel@vger.kernel.org>
Subject: Re: [PATCH 1/2] kbuild: remove cc-option test of -fno-stack-protector
Date: Sat, 27 Jun 2020 05:00:12 +0900 [thread overview]
Message-ID: <CAK7LNASx-kj=fHFN9=NDB-HKRSdWAEpGyKF86q8127SqFc07Ag@mail.gmail.com> (raw)
In-Reply-To: <CAKwvOdnK4KZaE=D40UyKHN-nB1Y-oXXJUcVv08cGJNsExOs-Pw@mail.gmail.com>
On Sat, Jun 27, 2020 at 4:09 AM Nick Desaulniers
<ndesaulniers@google.com> wrote:
>
> On Fri, Jun 26, 2020 at 12:00 PM Masahiro Yamada <masahiroy@kernel.org> wrote:
> >
> > Some Makefiles already pass -fno-stack-protector unconditionally.
> > For example, arch/arm64/kernel/vdso/Makefile, arch/x86/xen/Makefile.
> > No problem report so far about hard-coding this option. So, we can
> > assume all supported compilers know -fno-stack-protector.
> >
> > GCC 4.8 and Clang support this option (https://godbolt.org/z/_HDGzN)
> >
> > Get rid of cc-option from -fno-stack-protector.
> >
> > Remove CONFIG_CC_HAS_STACKPROTECTOR_NONE, which should always be 'y'.
> >
> > Note:
> > arch/mips/vdso/Makefile adds -fno-stack-protector twice, first
> > unconditionally, and second conditionally. I removed the second one.
> >
> > Signed-off-by: Masahiro Yamada <masahiroy@kernel.org>
> > ---
> >
> > Documentation/kbuild/kconfig-language.rst | 4 ++--
> > Makefile | 2 +-
> > arch/Kconfig | 3 ---
> > arch/arm/boot/compressed/Makefile | 3 +--
> > arch/mips/vdso/Makefile | 3 +--
> > arch/powerpc/kernel/Makefile | 2 +-
> > arch/powerpc/platforms/powermac/Makefile | 2 +-
> > arch/sparc/vdso/Makefile | 4 ++--
> > arch/um/Makefile | 3 +--
> > arch/x86/Makefile | 2 +-
> > arch/x86/boot/compressed/Makefile | 2 +-
> > arch/x86/entry/vdso/Makefile | 4 ++--
> > arch/x86/kernel/cpu/Makefile | 3 +--
> > arch/x86/lib/Makefile | 2 +-
> > arch/x86/mm/Makefile | 7 +++----
> > arch/x86/power/Makefile | 3 +--
> > arch/x86/purgatory/Makefile | 2 +-
> > arch/x86/um/vdso/Makefile | 2 +-
> > arch/x86/xen/Makefile | 5 ++---
> > drivers/firmware/efi/libstub/Makefile | 2 +-
> > drivers/xen/Makefile | 3 +--
> > kernel/kcsan/Makefile | 3 +--
> > lib/Makefile | 4 ++--
> > mm/kasan/Makefile | 2 +-
> > 24 files changed, 30 insertions(+), 42 deletions(-)
> >
> > diff --git a/Documentation/kbuild/kconfig-language.rst b/Documentation/kbuild/kconfig-language.rst
> > index a1601ec3317b..2538e7cb08e6 100644
> > --- a/Documentation/kbuild/kconfig-language.rst
> > +++ b/Documentation/kbuild/kconfig-language.rst
> > @@ -540,8 +540,8 @@ followed by a test macro::
> > If you need to expose a compiler capability to makefiles and/or C source files,
> > `CC_HAS_` is the recommended prefix for the config option::
> >
> > - config CC_HAS_STACKPROTECTOR_NONE
> > - def_bool $(cc-option,-fno-stack-protector)
> > + config CC_HAS_ASM_GOTO
> > + def_bool $(success,$(srctree)/scripts/gcc-goto.sh $(CC))
> >
> > Build as module only
> > ~~~~~~~~~~~~~~~~~~~~
> > diff --git a/Makefile b/Makefile
> > index 5496a32dffa6..73948798ce3f 100644
> > --- a/Makefile
> > +++ b/Makefile
> > @@ -762,7 +762,7 @@ ifneq ($(CONFIG_FRAME_WARN),0)
> > KBUILD_CFLAGS += -Wframe-larger-than=$(CONFIG_FRAME_WARN)
> > endif
> >
> > -stackp-flags-$(CONFIG_CC_HAS_STACKPROTECTOR_NONE) := -fno-stack-protector
> > +stackp-flags-y := -fno-stack-protector
> > stackp-flags-$(CONFIG_STACKPROTECTOR) := -fstack-protector
> > stackp-flags-$(CONFIG_STACKPROTECTOR_STRONG) := -fstack-protector-strong
> >
> > diff --git a/arch/Kconfig b/arch/Kconfig
> > index 8cc35dc556c7..1ea61290900a 100644
> > --- a/arch/Kconfig
> > +++ b/arch/Kconfig
> > @@ -478,9 +478,6 @@ config HAVE_STACKPROTECTOR
> > An arch should select this symbol if:
> > - it has implemented a stack canary (e.g. __stack_chk_guard)
> >
> > -config CC_HAS_STACKPROTECTOR_NONE
> > - def_bool $(cc-option,-fno-stack-protector)
> > -
> > config STACKPROTECTOR
> > bool "Stack Protector buffer overflow detection"
> > depends on HAVE_STACKPROTECTOR
> > diff --git a/arch/arm/boot/compressed/Makefile b/arch/arm/boot/compressed/Makefile
> > index 00602a6fba04..cb7a56c6723c 100644
> > --- a/arch/arm/boot/compressed/Makefile
> > +++ b/arch/arm/boot/compressed/Makefile
> > @@ -84,9 +84,8 @@ endif
> >
> > # -fstack-protector-strong triggers protection checks in this code,
> > # but it is being used too early to link to meaningful stack_chk logic.
> > -nossp-flags-$(CONFIG_CC_HAS_STACKPROTECTOR_NONE) := -fno-stack-protector
> > $(foreach o, $(libfdt_objs) atags_to_fdt.o, \
> > - $(eval CFLAGS_$(o) := -I $(srctree)/scripts/dtc/libfdt $(nossp-flags-y)))
> > + $(eval CFLAGS_$(o) := -I $(srctree)/scripts/dtc/libfdt -fno-stack-protector))
> >
> > # These were previously generated C files. When you are building the kernel
> > # with O=, make sure to remove the stale files in the output tree. Otherwise,
> > diff --git a/arch/mips/vdso/Makefile b/arch/mips/vdso/Makefile
> > index 2e64c7600eea..57fe83235281 100644
> > --- a/arch/mips/vdso/Makefile
> > +++ b/arch/mips/vdso/Makefile
> > @@ -35,8 +35,7 @@ cflags-vdso := $(ccflags-vdso) \
> > -O3 -g -fPIC -fno-strict-aliasing -fno-common -fno-builtin -G 0 \
> > -mrelax-pic-calls $(call cc-option, -mexplicit-relocs) \
> > -fno-stack-protector -fno-jump-tables -DDISABLE_BRANCH_PROFILING \
> > - $(call cc-option, -fno-asynchronous-unwind-tables) \
> > - $(call cc-option, -fno-stack-protector)
> > + $(call cc-option, -fno-asynchronous-unwind-tables)
> > aflags-vdso := $(ccflags-vdso) \
> > -D__ASSEMBLY__ -Wa,-gdwarf-2
> >
> > diff --git a/arch/powerpc/kernel/Makefile b/arch/powerpc/kernel/Makefile
> > index 244542ae2a91..3a83f2b876a5 100644
> > --- a/arch/powerpc/kernel/Makefile
> > +++ b/arch/powerpc/kernel/Makefile
> > @@ -16,7 +16,7 @@ CFLAGS_prom_init.o += $(DISABLE_LATENT_ENTROPY_PLUGIN)
> > CFLAGS_btext.o += $(DISABLE_LATENT_ENTROPY_PLUGIN)
> > CFLAGS_prom.o += $(DISABLE_LATENT_ENTROPY_PLUGIN)
> >
> > -CFLAGS_prom_init.o += $(call cc-option, -fno-stack-protector)
> > +CFLAGS_prom_init.o += -fno-stack-protector
> > CFLAGS_prom_init.o += -DDISABLE_BRANCH_PROFILING
> > CFLAGS_prom_init.o += -ffreestanding
> >
> > diff --git a/arch/powerpc/platforms/powermac/Makefile b/arch/powerpc/platforms/powermac/Makefile
> > index f4247ade71ca..cf85f0662d0d 100644
> > --- a/arch/powerpc/platforms/powermac/Makefile
> > +++ b/arch/powerpc/platforms/powermac/Makefile
> > @@ -1,6 +1,6 @@
> > # SPDX-License-Identifier: GPL-2.0
> > CFLAGS_bootx_init.o += -fPIC
> > -CFLAGS_bootx_init.o += $(call cc-option, -fno-stack-protector)
> > +CFLAGS_bootx_init.o += -fno-stack-protector
> >
> > KASAN_SANITIZE_bootx_init.o := n
> >
> > diff --git a/arch/sparc/vdso/Makefile b/arch/sparc/vdso/Makefile
> > index 708cb6304c2d..f44355e46f31 100644
> > --- a/arch/sparc/vdso/Makefile
> > +++ b/arch/sparc/vdso/Makefile
> > @@ -54,7 +54,7 @@ $(obj)/vdso-image-%.c: $(obj)/vdso%.so.dbg $(obj)/vdso%.so $(obj)/vdso2c FORCE
> > # optimize sibling calls.
> > #
> > CFL := $(PROFILING) -mcmodel=medlow -fPIC -O2 -fasynchronous-unwind-tables -m64 \
> > - $(filter -g%,$(KBUILD_CFLAGS)) $(call cc-option, -fno-stack-protector) \
> > + $(filter -g%,$(KBUILD_CFLAGS)) -fno-stack-protector \
> > -fno-omit-frame-pointer -foptimize-sibling-calls \
> > -DDISABLE_BRANCH_PROFILING -DBUILD_VDSO
> >
> > @@ -93,7 +93,7 @@ KBUILD_CFLAGS_32 := $(filter-out -fno-pic,$(KBUILD_CFLAGS_32))
> > KBUILD_CFLAGS_32 := $(filter-out $(GCC_PLUGINS_CFLAGS),$(KBUILD_CFLAGS_32))
> > KBUILD_CFLAGS_32 := $(filter-out $(SPARC_REG_CFLAGS),$(KBUILD_CFLAGS_32))
> > KBUILD_CFLAGS_32 += -m32 -msoft-float -fpic
> > -KBUILD_CFLAGS_32 += $(call cc-option, -fno-stack-protector)
> > +KBUILD_CFLAGS_32 += -fno-stack-protector
> > KBUILD_CFLAGS_32 += $(call cc-option, -foptimize-sibling-calls)
> > KBUILD_CFLAGS_32 += -fno-omit-frame-pointer
> > KBUILD_CFLAGS_32 += -DDISABLE_BRANCH_PROFILING
> > diff --git a/arch/um/Makefile b/arch/um/Makefile
> > index 3f27aa3ec0a6..1cea46ff9bb7 100644
> > --- a/arch/um/Makefile
> > +++ b/arch/um/Makefile
> > @@ -121,8 +121,7 @@ LINK-$(CONFIG_LD_SCRIPT_STATIC) += -static
> > LINK-$(CONFIG_LD_SCRIPT_DYN) += -Wl,-rpath,/lib $(call cc-option, -no-pie)
> >
> > CFLAGS_NO_HARDENING := $(call cc-option, -fno-PIC,) $(call cc-option, -fno-pic,) \
> > - $(call cc-option, -fno-stack-protector,) \
> > - $(call cc-option, -fno-stack-protector-all,)
> > + -fno-stack-protector $(call cc-option, -fno-stack-protector-all)
>
> Just curious, looks like we could do the same for
> `-fno-stack-protector-all`, here or tree-wide, right? Wait, what
> compiler recognizes -fno-stack-protector-all?
> https://godbolt.org/z/QFQKE_
-fstack-protector
-fstack-protector-strong
-fstack-protector-all
are supported.
But,
-fno-stack-protector-strong
-fno-stack-protector-all
are unsupported.
Perheps, -fno-stack-protector is enough
to disable all variants of stack-protector.
--
Best Regards
Masahiro Yamada
next prev parent reply other threads:[~2020-06-26 20:01 UTC|newest]
Thread overview: 20+ messages / expand[flat|nested] mbox.gz Atom feed top
2020-06-26 18:59 [PATCH 1/2] kbuild: remove cc-option test of -fno-stack-protector Masahiro Yamada
2020-06-26 18:59 ` [PATCH 2/2] kbuild: remove cc-option test of -ffreestanding Masahiro Yamada
2020-06-26 19:58 ` Nick Desaulniers
2020-06-26 20:22 ` Kees Cook
2020-06-27 7:39 ` Ard Biesheuvel
2020-06-26 19:09 ` [PATCH 1/2] kbuild: remove cc-option test of -fno-stack-protector Nick Desaulniers
2020-06-26 20:00 ` Masahiro Yamada [this message]
2020-06-26 20:13 ` Kees Cook
2020-06-26 20:13 ` Nick Desaulniers
2020-06-26 20:21 ` Kees Cook
2020-06-26 20:25 ` Nick Desaulniers
2020-06-26 20:37 ` Kees Cook
2020-06-27 11:58 ` Masahiro Yamada
2020-06-29 18:26 ` Nick Desaulniers
2020-06-29 22:39 ` Nick Desaulniers
2020-06-30 18:18 ` Masahiro Yamada
2020-07-01 19:33 ` Masahiro Yamada
2020-06-26 20:18 ` Kees Cook
2020-06-27 7:39 ` Ard Biesheuvel
2020-07-01 6:01 ` Marco Elver
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to='CAK7LNASx-kj=fHFN9=NDB-HKRSdWAEpGyKF86q8127SqFc07Ag@mail.gmail.com' \
--to=masahiroy@kernel.org \
--cc=clang-built-linux@googlegroups.com \
--cc=keescook@chromium.org \
--cc=linux-efi@vger.kernel.org \
--cc=linux-kbuild@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=ndesaulniers@google.com \
--cc=x86@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.