linux-arm-kernel.lists.infradead.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 0/5] enablement of some security features missing on ARM
@ 2010-06-16 20:33 Nicolas Pitre
  2010-06-16 20:33 ` [PATCH 1/5] [ARM] implement arch_randomize_brk() Nicolas Pitre
                   ` (4 more replies)
  0 siblings, 5 replies; 11+ messages in thread
From: Nicolas Pitre @ 2010-06-16 20:33 UTC (permalink / raw)
  To: linux-arm-kernel

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 1/5] [ARM] implement arch_randomize_brk()
  2010-06-16 20:33 [PATCH 0/5] enablement of some security features missing on ARM Nicolas Pitre
@ 2010-06-16 20:33 ` Nicolas Pitre
  2010-06-18 10:55   ` Sergei Shtylyov
  2010-06-16 20:33 ` [PATCH 2/5] [ARM] add address randomization to mmap() Nicolas Pitre
                   ` (3 subsequent siblings)
  4 siblings, 1 reply; 11+ messages in thread
From: Nicolas Pitre @ 2010-06-16 20:33 UTC (permalink / raw)
  To: linux-arm-kernel

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.

Signed-off-by: Nicolas Pitre <nicolas.pitre@linaro.org>
---
 arch/arm/include/asm/elf.h |    3 +++
 arch/arm/kernel/process.c  |    7 +++++++
 2 files changed, 10 insertions(+), 0 deletions(-)

diff --git a/arch/arm/include/asm/elf.h b/arch/arm/include/asm/elf.h
index 51662fe..0a96e8c 100644
--- a/arch/arm/include/asm/elf.h
+++ b/arch/arm/include/asm/elf.h
@@ -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
diff --git a/arch/arm/kernel/process.c b/arch/arm/kernel/process.c
index acf5e6f..1c6eb7e 100644
--- a/arch/arm/kernel/process.c
+++ b/arch/arm/kernel/process.c
@@ -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

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 2/5] [ARM] add address randomization to mmap()
  2010-06-16 20:33 [PATCH 0/5] enablement of some security features missing on ARM Nicolas Pitre
  2010-06-16 20:33 ` [PATCH 1/5] [ARM] implement arch_randomize_brk() Nicolas Pitre
@ 2010-06-16 20:33 ` Nicolas Pitre
  2010-06-16 23:42   ` Kyungmin Park
  2010-06-16 20:33 ` [PATCH 3/5] ARM: initial stack protector (-fstack-protector) support Nicolas Pitre
                   ` (2 subsequent siblings)
  4 siblings, 1 reply; 11+ messages in thread
From: Nicolas Pitre @ 2010-06-16 20:33 UTC (permalink / raw)
  To: linux-arm-kernel

Signed-off-by: Nicolas Pitre <nicolas.pitre@linaro.org>
---
 arch/arm/mm/mmap.c |    4 ++++
 1 files changed, 4 insertions(+), 0 deletions(-)

diff --git a/arch/arm/mm/mmap.c b/arch/arm/mm/mmap.c
index f5abc51..4f5b396 100644
--- a/arch/arm/mm/mmap.c
+++ b/arch/arm/mm/mmap.c
@@ -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

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 3/5] ARM: initial stack protector (-fstack-protector) support
  2010-06-16 20:33 [PATCH 0/5] enablement of some security features missing on ARM Nicolas Pitre
  2010-06-16 20:33 ` [PATCH 1/5] [ARM] implement arch_randomize_brk() Nicolas Pitre
  2010-06-16 20:33 ` [PATCH 2/5] [ARM] add address randomization to mmap() Nicolas Pitre
@ 2010-06-16 20:33 ` Nicolas Pitre
  2010-06-16 20:33 ` [PATCH 4/5] ARM: stack protector: change the canary value per task Nicolas Pitre
  2010-06-16 20:33 ` [PATCH 5/5] Stack protector: test module Nicolas Pitre
  4 siblings, 0 replies; 11+ messages in thread
From: Nicolas Pitre @ 2010-06-16 20:33 UTC (permalink / raw)
  To: linux-arm-kernel

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.

Signed-off-by: Nicolas Pitre <nicolas.pitre@linaro.org>
---
 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

diff --git a/arch/arm/Kconfig b/arch/arm/Kconfig
index 1f254bd..f160b93 100644
--- a/arch/arm/Kconfig
+++ b/arch/arm/Kconfig
@@ -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"
diff --git a/arch/arm/Makefile b/arch/arm/Makefile
index 64ba313..ddf6da1 100644
--- a/arch/arm/Makefile
+++ b/arch/arm/Makefile
@@ -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
diff --git a/arch/arm/include/asm/stackprotector.h b/arch/arm/include/asm/stackprotector.h
new file mode 100644
index 0000000..de00332
--- /dev/null
+++ b/arch/arm/include/asm/stackprotector.h
@@ -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 */
diff --git a/arch/arm/kernel/process.c b/arch/arm/kernel/process.c
index 1c6eb7e..090ac94 100644
--- a/arch/arm/kernel/process.c
+++ b/arch/arm/kernel/process.c
@@ -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

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 4/5] ARM: stack protector: change the canary value per task
  2010-06-16 20:33 [PATCH 0/5] enablement of some security features missing on ARM Nicolas Pitre
                   ` (2 preceding siblings ...)
  2010-06-16 20:33 ` [PATCH 3/5] ARM: initial stack protector (-fstack-protector) support Nicolas Pitre
@ 2010-06-16 20:33 ` Nicolas Pitre
  2010-06-16 20:33 ` [PATCH 5/5] Stack protector: test module Nicolas Pitre
  4 siblings, 0 replies; 11+ messages in thread
From: Nicolas Pitre @ 2010-06-16 20:33 UTC (permalink / raw)
  To: linux-arm-kernel

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.

Signed-off-by: Nicolas Pitre <nicolas.pitre@linaro.org>
---
 arch/arm/kernel/asm-offsets.c |    3 +++
 arch/arm/kernel/entry-armv.S  |    8 ++++++++
 2 files changed, 11 insertions(+), 0 deletions(-)

diff --git a/arch/arm/kernel/asm-offsets.c b/arch/arm/kernel/asm-offsets.c
index 8835115..85f2a01 100644
--- a/arch/arm/kernel/asm-offsets.c
+++ b/arch/arm/kernel/asm-offsets.c
@@ -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));
diff --git a/arch/arm/kernel/entry-armv.S b/arch/arm/kernel/entry-armv.S
index 7ee48e7..2d14081 100644
--- a/arch/arm/kernel/entry-armv.S
+++ b/arch/arm/kernel/entry-armv.S
@@ -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

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 5/5] Stack protector: test module
  2010-06-16 20:33 [PATCH 0/5] enablement of some security features missing on ARM Nicolas Pitre
                   ` (3 preceding siblings ...)
  2010-06-16 20:33 ` [PATCH 4/5] ARM: stack protector: change the canary value per task Nicolas Pitre
@ 2010-06-16 20:33 ` Nicolas Pitre
  4 siblings, 0 replies; 11+ messages in thread
From: Nicolas Pitre @ 2010-06-16 20:33 UTC (permalink / raw)
  To: linux-arm-kernel

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

Signed-off-by: Nicolas Pitre <nicolas.pitre@linaro.org>
---
 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

diff --git a/samples/Kconfig b/samples/Kconfig
index 8924f72..13f0eb2 100644
--- a/samples/Kconfig
+++ b/samples/Kconfig
@@ -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
diff --git a/samples/Makefile b/samples/Makefile
index 0f15e6d..69132eb 100644
--- a/samples/Makefile
+++ b/samples/Makefile
@@ -1,4 +1,4 @@
 # Makefile for Linux samples code
 
 obj-$(CONFIG_SAMPLES)	+= kobject/ kprobes/ tracepoints/ trace_events/ \
-			   hw_breakpoint/
+			   hw_breakpoint/ tests/
diff --git a/samples/tests/Makefile b/samples/tests/Makefile
new file mode 100644
index 0000000..b174287
--- /dev/null
+++ b/samples/tests/Makefile
@@ -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
diff --git a/samples/tests/test_stackprotector.c b/samples/tests/test_stackprotector.c
new file mode 100644
index 0000000..1ea19a7
--- /dev/null
+++ b/samples/tests/test_stackprotector.c
@@ -0,0 +1,86 @@
+/*
+ * Sample test module to exercize the -fstack-protector buffer overflow
+ * detection feature.
+ *
+ * Author:	Nicolas Pitre
+ * Created:	June 7, 2010
+ * Copyright:	Canonical Ltd
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License version 2 as
+ * published by the Free Software Foundation.
+ *
+ * Beware: upon insertion of this module, the kernel should panic!
+ */
+
+#include <linux/kernel.h>
+#include <linux/module.h>
+
+static long (*test_fn)(long *);
+
+static long test_good(long *x)
+{
+	int i;
+	long v;
+
+	x[0] = 0;
+	x[1] = 1;
+	for (i = 2; i < 50; i++) {
+		v = x[i-1] + x[i-2];
+		x[i] = v;
+	}
+
+	return v;
+}
+
+static void test_bad_target(void)
+{
+	/* Execution should never get here. */
+	panic("*** FAIL: STACK OVERFLOW DETECTION DID NOT TRIGGER ***\n");
+}
+
+static long test_stack_attack(long *x)
+{
+	int i;
+
+	/*
+	 * Let's overwrite the stack to scrub over our caller's
+	 * own return address.
+	 */
+	for (i = 50; i < 70; i++)
+		x[i] = (long)&test_bad_target;
+
+	/*
+	 * And then pretend some normality.
+	 */
+	return test_good(x);
+}
+
+/* this is not marked "static" to make sure it is not inlined */
+long test_stackprotected_caller(long *x)
+{
+	long buffer[50];
+	return test_fn(buffer);
+}
+
+static int __init test_stackprotector_init(void)
+{
+	long dummy_buffer[20];
+
+	printk(KERN_CRIT "*** stack protector test module ***\n");
+
+	test_fn = &test_good;
+	printk(KERN_CRIT "... testing normal function call\n");
+	test_stackprotected_caller(dummy_buffer);
+
+	test_fn = &test_stack_attack;
+	printk(KERN_CRIT "... testing rogue function call\n");
+	test_stackprotected_caller(dummy_buffer);
+
+	/* the kernel should have panicked by now */
+	printk(KERN_CRIT "*** FAIL: stack overflow attempt did not work ***\n");
+	return -EINVAL;
+}
+
+module_init(test_stackprotector_init)
+MODULE_LICENSE("GPL");
-- 
1.7.1.337.gbd0bc

^ permalink raw reply related	[flat|nested] 11+ messages in thread

* [PATCH 2/5] [ARM] add address randomization to mmap()
  2010-06-16 20:33 ` [PATCH 2/5] [ARM] add address randomization to mmap() Nicolas Pitre
@ 2010-06-16 23:42   ` Kyungmin Park
  2010-06-17  0:05     ` Nicolas Pitre
  0 siblings, 1 reply; 11+ messages in thread
From: Kyungmin Park @ 2010-06-16 23:42 UTC (permalink / raw)
  To: linux-arm-kernel

Hi,

On Thu, Jun 17, 2010 at 5:33 AM, Nicolas Pitre <nico@fluxnic.net> wrote:
> Signed-off-by: Nicolas Pitre <nicolas.pitre@linaro.org>
> ---
> ?arch/arm/mm/mmap.c | ? ?4 ++++
> ?1 files changed, 4 insertions(+), 0 deletions(-)
>
> diff --git a/arch/arm/mm/mmap.c b/arch/arm/mm/mmap.c
> index f5abc51..4f5b396 100644
> --- a/arch/arm/mm/mmap.c
> +++ b/arch/arm/mm/mmap.c
> @@ -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;

Doesn't better to use mask operation?
                   addr += (get_random_int() & ((1 << 8) - 1)) << PAGE_SHIFT;

Thank you,
Kyungmin Park
>
> ?full_search:
> ? ? ? ?if (do_align)
> --
> 1.7.1.337.gbd0bc
>
>
> _______________________________________________
> linux-arm-kernel mailing list
> linux-arm-kernel at lists.infradead.org
> http://lists.infradead.org/mailman/listinfo/linux-arm-kernel
>

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 2/5] [ARM] add address randomization to mmap()
  2010-06-16 23:42   ` Kyungmin Park
@ 2010-06-17  0:05     ` Nicolas Pitre
  2010-06-17  0:25       ` Kyungmin Park
  0 siblings, 1 reply; 11+ messages in thread
From: Nicolas Pitre @ 2010-06-17  0:05 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, 17 Jun 2010, Kyungmin Park wrote:

> Hi,
> 
> On Thu, Jun 17, 2010 at 5:33 AM, Nicolas Pitre <nico@fluxnic.net> wrote:
> > Signed-off-by: Nicolas Pitre <nicolas.pitre@linaro.org>
> > ---
> > ?arch/arm/mm/mmap.c | ? ?4 ++++
> > ?1 files changed, 4 insertions(+), 0 deletions(-)
> >
> > diff --git a/arch/arm/mm/mmap.c b/arch/arm/mm/mmap.c
> > index f5abc51..4f5b396 100644
> > --- a/arch/arm/mm/mmap.c
> > +++ b/arch/arm/mm/mmap.c
> > @@ -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;
> 
> Doesn't better to use mask operation?
>                    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

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 2/5] [ARM] add address randomization to mmap()
  2010-06-17  0:05     ` Nicolas Pitre
@ 2010-06-17  0:25       ` Kyungmin Park
  0 siblings, 0 replies; 11+ messages in thread
From: Kyungmin Park @ 2010-06-17  0:25 UTC (permalink / raw)
  To: linux-arm-kernel

On Thu, Jun 17, 2010 at 9:05 AM, Nicolas Pitre <nico@fluxnic.net> wrote:
> On Thu, 17 Jun 2010, Kyungmin Park wrote:
>
>> Hi,
>>
>> On Thu, Jun 17, 2010 at 5:33 AM, Nicolas Pitre <nico@fluxnic.net> wrote:
>> > Signed-off-by: Nicolas Pitre <nicolas.pitre@linaro.org>
>> > ---
>> > ?arch/arm/mm/mmap.c | ? ?4 ++++
>> > ?1 files changed, 4 insertions(+), 0 deletions(-)
>> >
>> > diff --git a/arch/arm/mm/mmap.c b/arch/arm/mm/mmap.c
>> > index f5abc51..4f5b396 100644
>> > --- a/arch/arm/mm/mmap.c
>> > +++ b/arch/arm/mm/mmap.c
>> > @@ -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;
>>
>> Doesn't better to use mask operation?
>> ? ? ? ? ? ? ? ? ? ?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.
>

Right, Good to know

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

Thank you,
Kyungmin Park

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 1/5] [ARM] implement arch_randomize_brk()
  2010-06-16 20:33 ` [PATCH 1/5] [ARM] implement arch_randomize_brk() Nicolas Pitre
@ 2010-06-18 10:55   ` Sergei Shtylyov
  2010-06-18 12:15     ` Jamie Lokier
  0 siblings, 1 reply; 11+ messages in thread
From: Sergei Shtylyov @ 2010-06-18 10:55 UTC (permalink / raw)
  To: linux-arm-kernel

Hello.

Nicolas Pitre wrote:

> 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.

> Signed-off-by: Nicolas Pitre <nicolas.pitre@linaro.org>
[...]
> diff --git a/arch/arm/include/asm/elf.h b/arch/arm/include/asm/elf.h
> index 51662fe..0a96e8c 100644
> --- a/arch/arm/include/asm/elf.h
> +++ b/arch/arm/include/asm/elf.h
> @@ -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

    Isn't this an "infinitely recursive" macro?

WBR, Sergei

^ permalink raw reply	[flat|nested] 11+ messages in thread

* [PATCH 1/5] [ARM] implement arch_randomize_brk()
  2010-06-18 10:55   ` Sergei Shtylyov
@ 2010-06-18 12:15     ` Jamie Lokier
  0 siblings, 0 replies; 11+ messages in thread
From: Jamie Lokier @ 2010-06-18 12:15 UTC (permalink / raw)
  To: linux-arm-kernel

Sergei Shtylyov wrote:
> >+extern unsigned long arch_randomize_brk(struct mm_struct *mm);
> >+#define arch_randomize_brk arch_randomize_brk
> 
>    Isn't this an "infinitely recursive" macro?

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

^ permalink raw reply	[flat|nested] 11+ messages in thread

end of thread, other threads:[~2010-06-18 12:15 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-06-16 20:33 [PATCH 0/5] enablement of some security features missing on ARM Nicolas Pitre
2010-06-16 20:33 ` [PATCH 1/5] [ARM] implement arch_randomize_brk() Nicolas Pitre
2010-06-18 10:55   ` Sergei Shtylyov
2010-06-18 12:15     ` Jamie Lokier
2010-06-16 20:33 ` [PATCH 2/5] [ARM] add address randomization to mmap() Nicolas Pitre
2010-06-16 23:42   ` Kyungmin Park
2010-06-17  0:05     ` Nicolas Pitre
2010-06-17  0:25       ` Kyungmin Park
2010-06-16 20:33 ` [PATCH 3/5] ARM: initial stack protector (-fstack-protector) support Nicolas Pitre
2010-06-16 20:33 ` [PATCH 4/5] ARM: stack protector: change the canary value per task Nicolas Pitre
2010-06-16 20:33 ` [PATCH 5/5] Stack protector: test module Nicolas Pitre

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).