enablement of some security features missing on ARM

Those are simple patches adding address space layout randomization for the user space heap and mmap(), as well as stack protector support.

The stack protector support depends on GCC's ability to insert a canary on the stack upon entering a function, and validating it before leaving that function. The included test module in patch #5 intends to test that, but (at least with the GCC version I have here) the canary code is not inserted in that particular test code unless it is forced with -fstack-protector-all (if anyone has a clue to why I'd be interested).

[PATCH 1/5] [ARM] implement arch_randomize_brk()
[PATCH 2/5] [ARM] add address randomization to mmap() [PATCH 3/5] ARM: initial stack protector (-fstack-protector) support [PATCH 4/5] ARM: stack protector: change the canary value per task [PATCH 5/5] Stack protector: test module

Nicolas

For this feature to take effect, CONFIG_COMPAT_BRK must be turned off. This can safely be turned off for any EABI user space versions.

---
 arch/arm/include/asm/elf.h |    3 +++
 arch/arm/kernel/process.c  |    7 +++++++
 2 files changed, 10 insertions(+), 0 deletions(-)

index 51662fe..0a96e8c 100644
@@ -121,4 +121,7 @@ int dump_task_regs(struct task_struct *t, elf_gregset_t *elfregs);
 extern void elf_set_personality(const struct elf32_hdr *);
 #define SET_PERSONALITY(ex)	elf_set_personality(&(ex))
 
+extern unsigned long arch_randomize_brk(struct mm_struct *mm);
+#define arch_randomize_brk arch_randomize_brk
+
 #endif

index acf5e6f..1c6eb7e 100644
@@ -28,6 +28,7 @@
 #include <linux/tick.h>
 #include <linux/utsname.h>
 #include <linux/uaccess.h>
+#include <linux/random.h>
 
 #include <asm/leds.h>
 #include <asm/processor.h>

@@ -421,3 +422,9 @@ unsigned long get_wchan(struct task_struct *p)
 	} while (count ++ < 16);
 	return 0;
 }
+
+unsigned long arch_randomize_brk(struct mm_struct *mm)
+{
+	unsigned long range_end = mm->brk + 0x02000000;
+	return randomize_range(mm->brk, range_end, 0) ? : mm->brk;
+}

1.7.1.337.gbd0bc

Hello.

[...]

Not in ISO C. It'll expand to itself and stop. This is a handy trick for confirming #ifdef tests elsewhere. Glibc does it a lot for enums defined in header files, for those which applications commonly test with #ifdef because they were traditionally defined using macros.

-- Jamie

---
 arch/arm/mm/mmap.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

index f5abc51..4f5b396 100644
@@ -7,6 +7,7 @@
 #include <linux/shm.h>
 #include <linux/sched.h>
 #include <linux/io.h>
+#include <linux/random.h>
 #include <asm/cputype.h>
 #include <asm/system.h>
 

@@ -80,6 +81,9 @@ arch_get_unmapped_area(struct file *filp, unsigned long addr,
 	        start_addr = addr = TASK_UNMAPPED_BASE;
 	        mm->cached_hole_size = 0;
 	}
+	/* 8 bits of randomness in 20 address space bits */
+	if (current->flags & PF_RANDOMIZE)
+		addr += (get_random_int() % (1 << 8)) << PAGE_SHIFT;
 
 full_search:
 	if (do_align)

1.7.1.337.gbd0bc

Hi,

                   addr += (get_random_int() & ((1 << 8) - 1)) << PAGE_SHIFT;

GCC is smart enough to optimize the modulus into a mask, effectively generating the exact same assembly code in both cases.

Nicolas

Right, Good to know

  1c:   e0822003        add     r2, r2, r3
  20:   e20220ff        and     r2, r2, #255    ; 0xff

Thank you,
Kyungmin Park

This is the very basic stuff without the changing canary upon task switch yet. Just the Kconfig option and a constant canary value initialized at boot time.

---
 arch/arm/Kconfig                      |   12 ++++++++++
 arch/arm/Makefile                     |    4 +++
 arch/arm/include/asm/stackprotector.h |   38 +++++++++++++++++++++++++++++++++
 arch/arm/kernel/process.c             |    6 +++++
 4 files changed, 60 insertions(+), 0 deletions(-)
 create mode 100644 arch/arm/include/asm/stackprotector.h

index 1f254bd..f160b93 100644
@@ -1374,6 +1374,18 @@ config UACCESS_WITH_MEMCPY
 	  However, if the CPU data cache is using a write-allocate mode,
 	  this option is unlikely to provide any performance gain.
 
+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.
+
 endmenu
 
 menu "Boot options"

index 64ba313..ddf6da1 100644
@@ -34,6 +34,10 @@ 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

new file mode 100644
index 0000000..de00332
@@ -0,0 +1,38 @@
+/*
+ * 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 */

index 1c6eb7e..090ac94 100644
@@ -37,6 +37,12 @@
 #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",

1.7.1.337.gbd0bc

A new random value for the canary is stored in the task struct whenever a new task is forked. This is meant to allow for different canary values per task. On ARM, GCC expects the canary value to be found in a global variable called __stack_chk_guard. So this variable has to be updated with the value stored in the task struct whenever a task switch occurs.

Because the variable GCC expects is global, this cannot work on SMP unfortunately. So, on SMP, the same initial canary value is kept throughout, making this feature a bit less effective although it is still useful.

One way to overcome this GCC limitation would be to locate the __stack_chk_guard variable into a memory page of its own for each CPU, and then use TLB locking to have each CPU see its own page at the same virtual address for each of them.

---
 arch/arm/kernel/asm-offsets.c |    3 +++
 arch/arm/kernel/entry-armv.S  |    8 ++++++++
 2 files changed, 11 insertions(+), 0 deletions(-)

index 8835115..85f2a01 100644
@@ -40,6 +40,9 @@
 int main(void)
 {
   DEFINE(TSK_ACTIVE_MM,		offsetof(struct task_struct, active_mm));
+#ifdef CONFIG_CC_STACKPROTECTOR
+  DEFINE(TSK_STACK_CANARY,	offsetof(struct task_struct, stack_canary));
+#endif
   BLANK();
   DEFINE(TI_FLAGS,		offsetof(struct thread_info, flags));
   DEFINE(TI_PREEMPT,		offsetof(struct thread_info, preempt_count));

index 7ee48e7..2d14081 100644
@@ -745,6 +745,11 @@ ENTRY(__switch_to)
 	mov	r4, #0xffff0fff
 	str	r3, [r4, #-15]			@ TLS val at 0xffff0ff0
 #endif
+#if defined(CONFIG_CC_STACKPROTECTOR) && !defined(CONFIG_SMP)
+	ldr	r7, [r2, #TI_TASK]
+	ldr	r8, =__stack_chk_guard
+	ldr	r7, [r7, #TSK_STACK_CANARY]
+#endif
 #ifdef CONFIG_MMU
 	mcr	p15, 0, r6, c3, c0, 0		@ Set domain register
 #endif

@@ -753,6 +758,9 @@ ENTRY(__switch_to)
 	ldr	r0, =thread_notify_head
 	mov	r1, #THREAD_NOTIFY_SWITCH
 	bl	atomic_notifier_call_chain
+#if defined(CONFIG_CC_STACKPROTECTOR) && !defined(CONFIG_SMP)
+	str	r7, [r8]
+#endif
  THUMB(	mov	ip, r4			   )
 	mov	r0, r5
  ARM(	ldmia	r4, {r4 - sl, fp, sp, pc}  )	@ Load all regs saved previously

1.7.1.337.gbd0bc

This module creates a stack overflow and tests if the stack protector actually catches it.

---
 samples/Kconfig                     |    8 +++
 samples/Makefile                    |    2 +-
 samples/tests/Makefile              |    5 ++
 samples/tests/test_stackprotector.c |   86 +++++++++++++++++++++++++++++++++++
 4 files changed, 100 insertions(+), 1 deletions(-)
 create mode 100644 samples/tests/Makefile
 create mode 100644 samples/tests/test_stackprotector.c

index 8924f72..13f0eb2 100644
@@ -44,4 +44,12 @@ config SAMPLE_HW_BREAKPOINT
 	help
 	  This builds kernel hardware breakpoint example modules.
 
+config SAMPLE_TEST_STACKPROTECTOR
+	tristate "Build test module for the stack protector -- loadable modules only"
+	depends on CC_STACKPROTECTOR && m
+	help
+	  This build a test module which upon insertion will exercize
+	  the -fstack-protector buffer overflow detection feature.
+	  Beware that this test is destructive as it will panic the kernel. 
+
 endif # SAMPLES

index 0f15e6d..69132eb 100644
@@ -1,4 +1,4 @@
 # Makefile for Linux samples code
 
 obj-$(CONFIG_SAMPLES)	+= kobject/ kprobes/ tracepoints/ trace_events/ \
-			   hw_breakpoint/
+			   hw_breakpoint/ tests/

new file mode 100644
index 0000000..b174287
@@ -0,0 +1,5 @@
+# builds those example test modules, then to use one
+# (as root):insmod <module_name.ko>
+# Beware: those tests may be destructive!
+
+obj-$(CONFIG_SAMPLE_TEST_STACKPROTECTOR) += test_stackprotector.o

1.7.1.337.gbd0bc