* [PATCH v3] ARM: fix atags_to_fdt with stack-protector-strong
@ 2016-01-06 23:36 Kees Cook
2016-01-07 10:39 ` Arnd Bergmann
2016-01-07 11:30 ` Russell King - ARM Linux
0 siblings, 2 replies; 6+ messages in thread
From: Kees Cook @ 2016-01-06 23:36 UTC (permalink / raw)
To: Arnd Bergmann
Cc: Russell King, Simon Horman, Geert Uytterhoeven, Laurent Pinchart,
Magnus Damm, linux-arm-kernel, linux-kernel
Building with CONFIG_CC_STACKPROTECTOR_STRONG triggers protection code
generation under CONFIG_ARM_ATAG_DTB_COMPAT but this is too early for
being able to use any of the stack_chk code. Explicitly disable it for
only the atags_to_fdt bits.
Suggested-by: zhxihu <zhxihu@marvell.com>
Signed-off-by: Kees Cook <keescook@chromium.org>
---
v3:
- actually send to everyone correctly
v2:
- use call cc-option unconditionally, arnd
---
arch/arm/boot/compressed/Makefile | 8 ++++++++
1 file changed, 8 insertions(+)
diff --git a/arch/arm/boot/compressed/Makefile b/arch/arm/boot/compressed/Makefile
index 3f9a9ebc77c3..d7d2c2981f65 100644
--- a/arch/arm/boot/compressed/Makefile
+++ b/arch/arm/boot/compressed/Makefile
@@ -106,6 +106,14 @@ ORIG_CFLAGS := $(KBUILD_CFLAGS)
KBUILD_CFLAGS = $(subst -pg, , $(ORIG_CFLAGS))
endif
+# -fstack-protector-strong triggers protection checks in this code,
+# but it is being used too early to link to meaningful stack_chk logic.
+CFLAGS_atags_to_fdt.o := $(call cc-option, -fno-stack-protector)
+CFLAGS_fdt.o := $(call cc-option, -fno-stack-protector)
+CFLAGS_fdt_ro.o := $(call cc-option, -fno-stack-protector)
+CFLAGS_fdt_rw.o := $(call cc-option, -fno-stack-protector)
+CFLAGS_fdt_wip.o := $(call cc-option, -fno-stack-protector)
+
ccflags-y := -fpic -mno-single-pic-base -fno-builtin -I$(obj)
asflags-y := -DZIMAGE
--
2.6.3
--
Kees Cook
Chrome OS & Brillo Security
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH v3] ARM: fix atags_to_fdt with stack-protector-strong
2016-01-06 23:36 [PATCH v3] ARM: fix atags_to_fdt with stack-protector-strong Kees Cook
@ 2016-01-07 10:39 ` Arnd Bergmann
2016-01-07 11:30 ` Russell King - ARM Linux
1 sibling, 0 replies; 6+ messages in thread
From: Arnd Bergmann @ 2016-01-07 10:39 UTC (permalink / raw)
To: Kees Cook
Cc: Russell King, Simon Horman, Geert Uytterhoeven, Laurent Pinchart,
Magnus Damm, linux-arm-kernel, linux-kernel
On Wednesday 06 January 2016 15:36:56 Kees Cook wrote:
> Building with CONFIG_CC_STACKPROTECTOR_STRONG triggers protection code
> generation under CONFIG_ARM_ATAG_DTB_COMPAT but this is too early for
> being able to use any of the stack_chk code. Explicitly disable it for
> only the atags_to_fdt bits.
>
> Suggested-by: zhxihu <zhxihu@marvell.com>
> Signed-off-by: Kees Cook <keescook@chromium.org>
>
Acked-by: Arnd Bergmann <arnd@arndb.de>
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3] ARM: fix atags_to_fdt with stack-protector-strong
2016-01-06 23:36 [PATCH v3] ARM: fix atags_to_fdt with stack-protector-strong Kees Cook
2016-01-07 10:39 ` Arnd Bergmann
@ 2016-01-07 11:30 ` Russell King - ARM Linux
2016-01-07 18:40 ` Kees Cook
1 sibling, 1 reply; 6+ messages in thread
From: Russell King - ARM Linux @ 2016-01-07 11:30 UTC (permalink / raw)
To: Kees Cook
Cc: Arnd Bergmann, Simon Horman, Geert Uytterhoeven,
Laurent Pinchart, Magnus Damm, linux-arm-kernel, linux-kernel
On Wed, Jan 06, 2016 at 03:36:56PM -0800, Kees Cook wrote:
> diff --git a/arch/arm/boot/compressed/Makefile b/arch/arm/boot/compressed/Makefile
> index 3f9a9ebc77c3..d7d2c2981f65 100644
> --- a/arch/arm/boot/compressed/Makefile
> +++ b/arch/arm/boot/compressed/Makefile
> @@ -106,6 +106,14 @@ ORIG_CFLAGS := $(KBUILD_CFLAGS)
> KBUILD_CFLAGS = $(subst -pg, , $(ORIG_CFLAGS))
> endif
>
> +# -fstack-protector-strong triggers protection checks in this code,
> +# but it is being used too early to link to meaningful stack_chk logic.
> +CFLAGS_atags_to_fdt.o := $(call cc-option, -fno-stack-protector)
> +CFLAGS_fdt.o := $(call cc-option, -fno-stack-protector)
> +CFLAGS_fdt_ro.o := $(call cc-option, -fno-stack-protector)
> +CFLAGS_fdt_rw.o := $(call cc-option, -fno-stack-protector)
> +CFLAGS_fdt_wip.o := $(call cc-option, -fno-stack-protector)
This will result in "$(call cc-option, -fno-stack-protector)" being
called five times when this Makefile is parsed, which seems very
wasteful. I'm sure there's better solutions to that - maybe caching
the value in a variable in a higher level makefile (eg,
arch/arm/Makefile) ?
Also, I suspect that all of the decompressor should be built with
-fno-stack-protector as we don't have sufficient environment here.
Maybe it should be placed in the global CFLAGS for the decompressor?
--
RMK's Patch system: http://www.arm.linux.org.uk/developer/patches/
FTTC broadband for 0.8mile line: currently at 9.6Mbps down 400kbps up
according to speedtest.net.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3] ARM: fix atags_to_fdt with stack-protector-strong
2016-01-07 11:30 ` Russell King - ARM Linux
@ 2016-01-07 18:40 ` Kees Cook
2016-01-07 19:11 ` Geert Uytterhoeven
0 siblings, 1 reply; 6+ messages in thread
From: Kees Cook @ 2016-01-07 18:40 UTC (permalink / raw)
To: Russell King - ARM Linux
Cc: Arnd Bergmann, Simon Horman, Geert Uytterhoeven,
Laurent Pinchart, Magnus Damm, linux-arm-kernel, LKML
On Thu, Jan 7, 2016 at 3:30 AM, Russell King - ARM Linux
<linux@arm.linux.org.uk> wrote:
> On Wed, Jan 06, 2016 at 03:36:56PM -0800, Kees Cook wrote:
>> diff --git a/arch/arm/boot/compressed/Makefile b/arch/arm/boot/compressed/Makefile
>> index 3f9a9ebc77c3..d7d2c2981f65 100644
>> --- a/arch/arm/boot/compressed/Makefile
>> +++ b/arch/arm/boot/compressed/Makefile
>> @@ -106,6 +106,14 @@ ORIG_CFLAGS := $(KBUILD_CFLAGS)
>> KBUILD_CFLAGS = $(subst -pg, , $(ORIG_CFLAGS))
>> endif
>>
>> +# -fstack-protector-strong triggers protection checks in this code,
>> +# but it is being used too early to link to meaningful stack_chk logic.
>> +CFLAGS_atags_to_fdt.o := $(call cc-option, -fno-stack-protector)
>> +CFLAGS_fdt.o := $(call cc-option, -fno-stack-protector)
>> +CFLAGS_fdt_ro.o := $(call cc-option, -fno-stack-protector)
>> +CFLAGS_fdt_rw.o := $(call cc-option, -fno-stack-protector)
>> +CFLAGS_fdt_wip.o := $(call cc-option, -fno-stack-protector)
>
> This will result in "$(call cc-option, -fno-stack-protector)" being
> called five times when this Makefile is parsed, which seems very
> wasteful. I'm sure there's better solutions to that - maybe caching
> the value in a variable in a higher level makefile (eg,
> arch/arm/Makefile) ?
Good point; I will adjust this to get a single invocation.
> Also, I suspect that all of the decompressor should be built with
> -fno-stack-protector as we don't have sufficient environment here.
> Maybe it should be placed in the global CFLAGS for the decompressor?
I prefer keeping it disabled in as narrow a range as possible. If
other code gains a level of complexity that it triggers the stack
protector code insertion, I think that's worth examining when it
happens. If this ever becomes an actual burden, then yeah, let's do it
for the whole decompressor, but I think it'd be best to revisit it if
it happens again.
-Kees
--
Kees Cook
Chrome OS & Brillo Security
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH v3] ARM: fix atags_to_fdt with stack-protector-strong
2016-01-07 18:40 ` Kees Cook
@ 2016-01-07 19:11 ` Geert Uytterhoeven
2016-01-07 19:16 ` Kees Cook
0 siblings, 1 reply; 6+ messages in thread
From: Geert Uytterhoeven @ 2016-01-07 19:11 UTC (permalink / raw)
To: Kees Cook
Cc: Russell King - ARM Linux, Arnd Bergmann, Simon Horman,
Geert Uytterhoeven, Laurent Pinchart, Magnus Damm,
linux-arm-kernel, LKML
Hi Kees,
On Thu, Jan 7, 2016 at 7:40 PM, Kees Cook <keescook@chromium.org> wrote:
>> Also, I suspect that all of the decompressor should be built with
>> -fno-stack-protector as we don't have sufficient environment here.
>> Maybe it should be placed in the global CFLAGS for the decompressor?
>
> I prefer keeping it disabled in as narrow a range as possible. If
> other code gains a level of complexity that it triggers the stack
> protector code insertion, I think that's worth examining when it
> happens. If this ever becomes an actual burden, then yeah, let's do it
> for the whole decompressor, but I think it'd be best to revisit it if
> it happens again.
What's the failure mode if the stack protector code insertion is triggered?
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] 6+ messages in thread* Re: [PATCH v3] ARM: fix atags_to_fdt with stack-protector-strong
2016-01-07 19:11 ` Geert Uytterhoeven
@ 2016-01-07 19:16 ` Kees Cook
0 siblings, 0 replies; 6+ messages in thread
From: Kees Cook @ 2016-01-07 19:16 UTC (permalink / raw)
To: Geert Uytterhoeven
Cc: Russell King - ARM Linux, Arnd Bergmann, Simon Horman,
Geert Uytterhoeven, Laurent Pinchart, Magnus Damm,
linux-arm-kernel, LKML
On Thu, Jan 7, 2016 at 11:11 AM, Geert Uytterhoeven
<geert@linux-m68k.org> wrote:
> Hi Kees,
>
> On Thu, Jan 7, 2016 at 7:40 PM, Kees Cook <keescook@chromium.org> wrote:
>>> Also, I suspect that all of the decompressor should be built with
>>> -fno-stack-protector as we don't have sufficient environment here.
>>> Maybe it should be placed in the global CFLAGS for the decompressor?
>>
>> I prefer keeping it disabled in as narrow a range as possible. If
>> other code gains a level of complexity that it triggers the stack
>> protector code insertion, I think that's worth examining when it
>> happens. If this ever becomes an actual burden, then yeah, let's do it
>> for the whole decompressor, but I think it'd be best to revisit it if
>> it happens again.
>
> What's the failure mode if the stack protector code insertion is triggered?
AIUI, this code shouldn't even link if the ssp code gets inserted
since it can't resolve __stack_chk_fail.
-Kees
--
Kees Cook
Chrome OS & Brillo Security
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2016-01-07 19:16 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-01-06 23:36 [PATCH v3] ARM: fix atags_to_fdt with stack-protector-strong Kees Cook
2016-01-07 10:39 ` Arnd Bergmann
2016-01-07 11:30 ` Russell King - ARM Linux
2016-01-07 18:40 ` Kees Cook
2016-01-07 19:11 ` Geert Uytterhoeven
2016-01-07 19:16 ` 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).