Re: [PATCH 1/2] stackprotector: add stack smashing protectorgeneric implementation

From: Carmelo AMOROSO
Date: Wed Jan 05 2011 - 07:00:07 EST


-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

On 12/23/2010 2:44 PM, Filippo ARCIDIACONO wrote:
> The current implementation of SSP used for ARM architecture is based
> on the global variable __stack_chk_guard
> (see commit c743f38013aeff58ef6252601e397b5ba281c633 by Nicolas Pitre).
> This implementation should be generic enough to be used for all
> architectures that rely upon that global variable (i.e. SH), so the
> ARM code can be widely used for all those architectures.
> The kbuild uses a script to check if the architecture does actually
> need to define the __stack_chk_guard, so it will use the generic SSP
> code, otherwise it will fall back to the architecture specific one
> (i.e. x86, x86_64).
>
> Signed-off-by: Filippo Arcidiacono <filippo.arcidiacono@xxxxxx>
> Signed-off-by: Carmelo Amoroso <carmelo.amoroso@xxxxxx>
> ---
> Makefile | 14 ++++++++-
> arch/Kconfig | 16 +++++++++++
> arch/arm/Kconfig | 12 --------
> arch/arm/Makefile | 4 ---
> arch/arm/include/asm/stackprotector.h | 38 ---------------------------
> arch/arm/kernel/process.c | 6 ----
> arch/x86/Kconfig | 16 -----------
> include/asm-generic/stackprotector.h | 39 ++++++++++++++++++++++++++++
> include/linux/stackprotector.h | 6 +++-
> init/main.c | 5 +++
> scripts/gcc-generic-has-stack-protector.sh | 9 ++++++
> 11 files changed, 86 insertions(+), 79 deletions(-)
> delete mode 100644 arch/arm/include/asm/stackprotector.h
> create mode 100644 include/asm-generic/stackprotector.h
> create mode 100755 scripts/gcc-generic-has-stack-protector.sh
>
> diff --git a/Makefile b/Makefile
> index 77044b7..7530650 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -550,8 +550,18 @@ KBUILD_CFLAGS += $(call cc-option,-Wframe-larger-than=${CONFIG_FRAME_WARN})
> endif
>
> # Force gcc to behave correct even for buggy distributions
> -ifndef CONFIG_CC_STACKPROTECTOR
> -KBUILD_CFLAGS += $(call cc-option, -fno-stack-protector)
> +ifdef CONFIG_CC_STACKPROTECTOR
> +stackp-y := $(call cc-option,-fstack-protector)
> +ifeq ($(stackp-y),)
> + $(warning stack protector enabled but no compiler support)
> +else
> +cc_has_sp := $(srctree)/scripts/gcc-generic-has-stack-protector.sh
> +ifeq ($(shell $(CONFIG_SHELL) $(cc_has_sp) $(CC)), y)
> + KBUILD_CFLAGS += $(stackp-y) -D__ARCH_WANT_STACK_CHK_GUARD
> +endif
> +endif
> +else
> + KBUILD_CFLAGS += $(call cc-option, -fno-stack-protector)
> endif
>
> ifdef CONFIG_FRAME_POINTER
> diff --git a/arch/Kconfig b/arch/Kconfig
> index 8bf0fa6..083245d 100644
> --- a/arch/Kconfig
> +++ b/arch/Kconfig
> @@ -175,4 +175,20 @@ config HAVE_PERF_EVENTS_NMI
> config HAVE_ARCH_JUMP_LABEL
> bool
>
> +config CC_STACKPROTECTOR
> + bool "Enable -fstack-protector buffer overflow detection"
> + help
> + This option turns on the -fstack-protector GCC feature. This
> + feature puts, at the beginning of functions, a canary value on
> + the stack just before the return address, and validates
> + the value just before actually returning. Stack based buffer
> + overflows (that need to overwrite this return address) now also
> + overwrite the canary, which gets detected and the attack is then
> + neutralized via a kernel panic.
> +
> + This feature requires gcc version 4.2 or above, or a distribution
> + gcc with the feature backported. Older versions are automatically
> + detected and for those versions, this configuration option is
> + ignored. (and a warning is printed during bootup)
> +
> source "kernel/gcov/Kconfig"
> diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
> index d56d21c..58c29a5 100644
> --- a/arch/arm/Kconfig
> +++ b/arch/arm/Kconfig
> @@ -1522,18 +1522,6 @@ config SECCOMP
> and the task is only allowed to execute a few safe syscalls
> defined by each seccomp mode.
>
> -config CC_STACKPROTECTOR
> - bool "Enable -fstack-protector buffer overflow detection (EXPERIMENTAL)"
> - help
> - This option turns on the -fstack-protector GCC feature. This
> - feature puts, at the beginning of functions, a canary value on
> - the stack just before the return address, and validates
> - the value just before actually returning. Stack based buffer
> - overflows (that need to overwrite this return address) now also
> - overwrite the canary, which gets detected and the attack is then
> - neutralized via a kernel panic.
> - This feature requires gcc version 4.2 or above.
> -
> config DEPRECATED_PARAM_STRUCT
> bool "Provide old way to pass kernel parameters"
> help
> diff --git a/arch/arm/Makefile b/arch/arm/Makefile
> index b87aed0..ebbd003 100644
> --- a/arch/arm/Makefile
> +++ b/arch/arm/Makefile
> @@ -37,10 +37,6 @@ ifeq ($(CONFIG_FRAME_POINTER),y)
> KBUILD_CFLAGS +=-fno-omit-frame-pointer -mapcs -mno-sched-prolog
> endif
>
> -ifeq ($(CONFIG_CC_STACKPROTECTOR),y)
> -KBUILD_CFLAGS +=-fstack-protector
> -endif
> -
> ifeq ($(CONFIG_CPU_BIG_ENDIAN),y)
> KBUILD_CPPFLAGS += -mbig-endian
> AS += -EB
> diff --git a/arch/arm/include/asm/stackprotector.h b/arch/arm/include/asm/stackprotector.h
> deleted file mode 100644
> index de00332..0000000
> --- a/arch/arm/include/asm/stackprotector.h
> +++ /dev/null
> @@ -1,38 +0,0 @@
> -/*
> - * GCC stack protector support.
> - *
> - * Stack protector works by putting predefined pattern at the start of
> - * the stack frame and verifying that it hasn't been overwritten when
> - * returning from the function. The pattern is called stack canary
> - * and gcc expects it to be defined by a global variable called
> - * "__stack_chk_guard" on ARM. This unfortunately means that on SMP
> - * we cannot have a different canary value per task.
> - */
> -
> -#ifndef _ASM_STACKPROTECTOR_H
> -#define _ASM_STACKPROTECTOR_H 1
> -
> -#include <linux/random.h>
> -#include <linux/version.h>
> -
> -extern unsigned long __stack_chk_guard;
> -
> -/*
> - * Initialize the stackprotector canary value.
> - *
> - * NOTE: this must only be called from functions that never return,
> - * and it must always be inlined.
> - */
> -static __always_inline void boot_init_stack_canary(void)
> -{
> - unsigned long canary;
> -
> - /* Try to get a semi random initial value. */
> - get_random_bytes(&canary, sizeof(canary));
> - canary ^= LINUX_VERSION_CODE;
> -
> - current->stack_canary = canary;
> - __stack_chk_guard = current->stack_canary;
> -}
> -
> -#endif /* _ASM_STACKPROTECTOR_H */
> diff --git a/arch/arm/kernel/process.c b/arch/arm/kernel/process.c
> index e76fcaa..067b4de 100644
> --- a/arch/arm/kernel/process.c
> +++ b/arch/arm/kernel/process.c
> @@ -39,12 +39,6 @@
> #include <asm/stacktrace.h>
> #include <asm/mach/time.h>
>
> -#ifdef CONFIG_CC_STACKPROTECTOR
> -#include <linux/stackprotector.h>
> -unsigned long __stack_chk_guard __read_mostly;
> -EXPORT_SYMBOL(__stack_chk_guard);
> -#endif
> -
> static const char *processor_modes[] = {
> "USER_26", "FIQ_26" , "IRQ_26" , "SVC_26" , "UK4_26" , "UK5_26" , "UK6_26" , "UK7_26" ,
> "UK8_26" , "UK9_26" , "UK10_26", "UK11_26", "UK12_26", "UK13_26", "UK14_26", "UK15_26",
> diff --git a/arch/x86/Kconfig b/arch/x86/Kconfig
> index e330da2..b8704dd 100644
> --- a/arch/x86/Kconfig
> +++ b/arch/x86/Kconfig
> @@ -1455,22 +1455,6 @@ config SECCOMP
>
> If unsure, say Y. Only embedded should say N here.
>
> -config CC_STACKPROTECTOR
> - bool "Enable -fstack-protector buffer overflow detection (EXPERIMENTAL)"
> - ---help---
> - This option turns on the -fstack-protector GCC feature. This
> - feature puts, at the beginning of functions, a canary value on
> - the stack just before the return address, and validates
> - the value just before actually returning. Stack based buffer
> - overflows (that need to overwrite this return address) now also
> - overwrite the canary, which gets detected and the attack is then
> - neutralized via a kernel panic.
> -
> - This feature requires gcc version 4.2 or above, or a distribution
> - gcc with the feature backported. Older versions are automatically
> - detected and for those versions, this configuration option is
> - ignored. (and a warning is printed during bootup)
> -
> source kernel/Kconfig.hz
>
> config KEXEC
> diff --git a/include/asm-generic/stackprotector.h b/include/asm-generic/stackprotector.h
> new file mode 100644
> index 0000000..2cf9c65
> --- /dev/null
> +++ b/include/asm-generic/stackprotector.h
> @@ -0,0 +1,39 @@
> +/*
> + * GCC stack protector support.
> + * (Generic implementation based on __stack_chk_guard)
> + *
> + * Stack protector works by putting predefined pattern at the start of
> + * the stack frame and verifying that it hasn't been overwritten when
> + * returning from the function. The pattern is called stack canary
> + * and gcc expects it to be defined by a global variable called
> + * "__stack_chk_guard". This unfortunately means that on SMP
> + * we cannot have a different canary value per task.
> + */
> +
> +#ifndef _LINUX_STACKPROTECTOR_H
> +#error "Never use <asm-generic/stackprotector.h> directly; \
> +include <linux/stackprotector.h> instead."
> +#endif
> +
> +#include <linux/random.h>
> +#include <linux/version.h>
> +
> +extern unsigned long __stack_chk_guard;
> +
> +/*
> + * Initialize the stackprotector canary value.
> + *
> + * NOTE: this must only be called from functions that never return,
> + * and it must always be inlined.
> + */
> +static __always_inline void boot_init_stack_canary(void)
> +{
> + unsigned long canary;
> +
> + /* Try to get a semi random initial value. */
> + get_random_bytes(&canary, sizeof(canary));
> + canary ^= LINUX_VERSION_CODE;
> +
> + current->stack_canary = canary;
> + __stack_chk_guard = current->stack_canary;
> +}
> diff --git a/include/linux/stackprotector.h b/include/linux/stackprotector.h
> index 6f3e54c..d71a437 100644
> --- a/include/linux/stackprotector.h
> +++ b/include/linux/stackprotector.h
> @@ -6,7 +6,11 @@
> #include <linux/random.h>
>
> #ifdef CONFIG_CC_STACKPROTECTOR
> -# include <asm/stackprotector.h>
> +#ifdef __ARCH_WANT_STACK_CHK_GUARD
> +#include <asm-generic/stackprotector.h>
> +#else
> +#include <asm/stackprotector.h>
> +#endif
> #else
> static inline void boot_init_stack_canary(void)
> {
> diff --git a/init/main.c b/init/main.c
> index 8646401..8b2e070 100644
> --- a/init/main.c
> +++ b/init/main.c
> @@ -78,6 +78,11 @@
> #include <asm/smp.h>
> #endif
>
> +#if defined CONFIG_CC_STACKPROTECTOR && defined __ARCH_WANT_STACK_CHK_GUARD
> +unsigned long __stack_chk_guard __read_mostly;
> +EXPORT_SYMBOL(__stack_chk_guard);
> +#endif
> +
> static int kernel_init(void *);
>
> extern void init_IRQ(void);
> diff --git a/scripts/gcc-generic-has-stack-protector.sh b/scripts/gcc-generic-has-stack-protector.sh
> new file mode 100755
> index 0000000..11cc914
> --- /dev/null
> +++ b/scripts/gcc-generic-has-stack-protector.sh
> @@ -0,0 +1,9 @@
> +#!/bin/sh
> +
> +echo "int foo(void) { char X[200]; return 3; }" | $1 -S -xc -c -O0 \
> +-fstack-protector - -o - 2> /dev/null | grep -q __stack_chk_guard
> +if [ "$?" -eq "0" ] ; then
> + echo y
> +else
> + echo n
> +fi

Is someone interested into this stuff ?

Thanks,
Carmelo
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (MingW32)
Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/

iEYEARECAAYFAk0kXRYACgkQoRq/3BrK1s+ZOwCfdZ80u8gdnA9fJikmL+uxsX5I
RWkAn2KUqB4F/RSQ6stRj4ajoPgvCRCa
=ub67
-----END PGP SIGNATURE-----
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/