[PATCH] ARM: decompressor: disable stack protector

[PATCH] ARM: decompressor: disable stack protector

[Ard Biesheuvel]

Enabling the stack protector in the decompressor is of dubious value,
given that it uses a fixed value for the canary, cannot print any output
unless CONFIG_DEBUG_LL is enabled (which relies on board specific build
time settings), and is already disabled for a good chunk of the code
(libfdt).

So let's just disable it in the decompressor. This will make it easier
in the future to manage the command line options that would need to be
removed again in this context for the TLS register based stack
protector.

Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 arch/arm/boot/compressed/Makefile | 6 +-----
 arch/arm/boot/compressed/misc.c   | 7 -------
 2 files changed, 1 insertion(+), 12 deletions(-)

diff --git a/arch/arm/boot/compressed/Makefile b/arch/arm/boot/compressed/Makefile
index 91265e7ff672..e2bd084b1cdf 100644
--- a/arch/arm/boot/compressed/Makefile
+++ b/arch/arm/boot/compressed/Makefile
@@ -93,11 +93,6 @@ ifeq ($(CONFIG_USE_OF),y)
 OBJS	+= $(libfdt_objs) fdt_check_mem_start.o
 endif
 
-# -fstack-protector-strong triggers protection checks in this code,
-# but it is being used too early to link to meaningful stack_chk logic.
-$(foreach o, $(libfdt_objs) atags_to_fdt.o fdt_check_mem_start.o, \
-	$(eval CFLAGS_$(o) := -I $(srctree)/scripts/dtc/libfdt -fno-stack-protector))
-
 targets       := vmlinux vmlinux.lds piggy_data piggy.o \
 		 lib1funcs.o ashldi3.o bswapsdi2.o \
 		 head.o $(OBJS)
@@ -107,6 +102,7 @@ clean-files += lib1funcs.S ashldi3.S bswapsdi2.S hyp-stub.S
 KBUILD_CFLAGS += -DDISABLE_BRANCH_PROFILING
 
 ccflags-y := -fpic $(call cc-option,-mno-single-pic-base,) -fno-builtin \
+	     -I$(srctree)/scripts/dtc/libfdt -fno-stack-protector \
 	     -I$(obj) $(DISABLE_ARM_SSP_PER_TASK_PLUGIN)
 ccflags-remove-$(CONFIG_FUNCTION_TRACER) += -pg
 asflags-y := -DZIMAGE
diff --git a/arch/arm/boot/compressed/misc.c b/arch/arm/boot/compressed/misc.c
index e1e9a5dde853..c3c66ff2d696 100644
--- a/arch/arm/boot/compressed/misc.c
+++ b/arch/arm/boot/compressed/misc.c
@@ -128,13 +128,6 @@ asmlinkage void __div0(void)
 	error("Attempting division by 0!");
 }
 
-const unsigned long __stack_chk_guard = 0x000a0dff;
-
-void __stack_chk_fail(void)
-{
-	error("stack-protector: Kernel stack is corrupted\n");
-}
-
 extern int do_decompress(u8 *input, int len, u8 *output, void (*error)(char *x));
 
 
-- 
2.30.2


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] ARM: decompressor: disable stack protector

[Kees Cook]

On Tue, Oct 26, 2021 at 10:27:52AM +0200, Ard Biesheuvel wrote:
> Enabling the stack protector in the decompressor is of dubious value,
> given that it uses a fixed value for the canary, cannot print any output
> unless CONFIG_DEBUG_LL is enabled (which relies on board specific build
> time settings), and is already disabled for a good chunk of the code
> (libfdt).
> 
> So let's just disable it in the decompressor. This will make it easier
> in the future to manage the command line options that would need to be
> removed again in this context for the TLS register based stack
> protector.
> 
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>

Yeah, that's fine. There's no good reason to complicate the decompressor
for the stack protector. If someone is trying to exploit the kernel at
this stage, the system has a much bigger problem. ;)

Acked-by: Kees Cook <keescook@chromium.org>

-- 
Kees Cook

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH] ARM: decompressor: disable stack protector

[Linus Walleij]

On Tue, Oct 26, 2021 at 10:28 AM Ard Biesheuvel <ardb@kernel.org> wrote:

> Enabling the stack protector in the decompressor is of dubious value,
> given that it uses a fixed value for the canary, cannot print any output
> unless CONFIG_DEBUG_LL is enabled (which relies on board specific build
> time settings), and is already disabled for a good chunk of the code
> (libfdt).
>
> So let's just disable it in the decompressor. This will make it easier
> in the future to manage the command line options that would need to be
> removed again in this context for the TLS register based stack
> protector.
>
> Signed-off-by: Ard Biesheuvel <ardb@kernel.org>

Makes sense.
Reviewed-by: Linus Walleij <linus.walleij@linaro.org>

Yours,
Linus Walleij

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel