Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1303150 > unrolled thread

[PATCH v3] ARM: fix atags_to_fdt with stack-protector-strong

Back to article view | Back to linux.kernel

Contents

  [PATCH v3] ARM: fix atags_to_fdt with stack-protector-strong Kees Cook <keescook@chromium.org> - 2016-01-07 00:40 +0100
    Re: [PATCH v3] ARM: fix atags_to_fdt with stack-protector-strong Arnd Bergmann <arnd@arndb.de> - 2016-01-07 11:50 +0100
    Re: [PATCH v3] ARM: fix atags_to_fdt with stack-protector-strong Russell King - ARM Linux <linux@arm.linux.org.uk> - 2016-01-07 12:40 +0100
      Re: [PATCH v3] ARM: fix atags_to_fdt with stack-protector-strong Kees Cook <keescook@chromium.org> - 2016-01-07 19:50 +0100
        Re: [PATCH v3] ARM: fix atags_to_fdt with stack-protector-strong Geert Uytterhoeven <geert@linux-m68k.org> - 2016-01-07 20:20 +0100
          Re: [PATCH v3] ARM: fix atags_to_fdt with stack-protector-strong Kees Cook <keescook@chromium.org> - 2016-01-07 20:20 +0100

#1303150 — [PATCH v3] ARM: fix atags_to_fdt with stack-protector-strong

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
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [next] | [standalone]

#1303471

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>
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1303500

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.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

[toc] | [prev] | [next] | [standalone]

#1303803

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

[toc] | [prev] | [next] | [standalone]

#1303814

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

[toc] | [prev] | [next] | [standalone]

#1303837

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

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web