diff mbox

UBUNTU: SAUCE: x86: brk away from exec rand area

Message ID 20100119183134.GP5185@outflux.net
State Accepted
Delegated to: Andy Whitcroft
Headers show

Commit Message

Kees Cook Jan. 19, 2010, 6:31 p.m. UTC
This is a fix for the NX emulation patch to force the brk area well
outside of the exec randomization area to avoid future allocation or brk
growth collisions.  Normally this isn't a problem, except when the text
region has been loaded from a PIE binary and the CS limit can't be put
just above bss.

Additionally, the nx-emulation patch was still randomizing addresses
even when randomize_va_space was disabled, which would cause collisions
even faster if someone tried to disable randomization.

BugLink: http://bugs.launchpad.net/bugs/452175

Signed-off-by: Kees Cook <kees.cook at canonical.com>
---
 arch/x86/kernel/process.c |   12 +++++++++++-
 mm/mmap.c                 |   11 ++++++++---
 2 files changed, 19 insertions(+), 4 deletions(-)

Comments

Andy Whitcroft Jan. 19, 2010, 6:40 p.m. UTC | #1
On Tue, Jan 19, 2010 at 10:31:34AM -0800, Kees Cook wrote:
> This is a fix for the NX emulation patch to force the brk area well
> outside of the exec randomization area to avoid future allocation or brk
> growth collisions.  Normally this isn't a problem, except when the text
> region has been loaded from a PIE binary and the CS limit can't be put
> just above bss.
> 
> Additionally, the nx-emulation patch was still randomizing addresses
> even when randomize_va_space was disabled, which would cause collisions
> even faster if someone tried to disable randomization.
> 
> BugLink: http://bugs.launchpad.net/bugs/452175
> 
> Signed-off-by: Kees Cook <kees.cook at canonical.com>

Acked-by: Andy Whitcroft <apw@canonical.com>

-apw
Stefan Bader Jan. 22, 2010, 2:04 p.m. UTC | #2
I d love to understand that foo better, maybe we can do a beer time scribble and
talk session in Portland. But it looks ok, we got it in Lucid without flames
erupting and it has been verified to fix the issue.

Kees Cook wrote:
> This is a fix for the NX emulation patch to force the brk area well
> outside of the exec randomization area to avoid future allocation or brk
> growth collisions.  Normally this isn't a problem, except when the text
> region has been loaded from a PIE binary and the CS limit can't be put
> just above bss.
> 
> Additionally, the nx-emulation patch was still randomizing addresses
> even when randomize_va_space was disabled, which would cause collisions
> even faster if someone tried to disable randomization.
> 
> BugLink: http://bugs.launchpad.net/bugs/452175
> 
> Signed-off-by: Kees Cook <kees.cook at canonical.com>
Acked-by: Stefan Bader <stefan.bader@canonical.com>
> ---
>  arch/x86/kernel/process.c |   12 +++++++++++-
>  mm/mmap.c                 |   11 ++++++++---
>  2 files changed, 19 insertions(+), 4 deletions(-)
> 
> diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
> index 5284cd2..bf5c156 100644
> --- a/arch/x86/kernel/process.c
> +++ b/arch/x86/kernel/process.c
> @@ -607,6 +607,16 @@ unsigned long arch_align_stack(unsigned long sp)
>  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;
> +	unsigned long bump = 0;
> +#ifdef CONFIG_X86_32
> +	/* in the case of NX emulation, shove the brk segment way out of the
> +           way of the exec randomization area, since it can collide with
> +	   future allocations if not. */
> +	if ( (mm->get_unmapped_exec_area == arch_get_unmapped_exec_area) &&
> +	     (mm->brk < 0x08000000) ) {
> +		bump = (TASK_SIZE/6);
> +	}
> +#endif
> +	return bump + (randomize_range(mm->brk, range_end, 0) ? : mm->brk);
>  }
>  
> diff --git a/mm/mmap.c b/mm/mmap.c
> index a1483c5..c6d7e53 100644
> --- a/mm/mmap.c
> +++ b/mm/mmap.c
> @@ -1500,8 +1500,11 @@ arch_get_unmapped_exec_area(struct file *filp, unsigned long addr0,
>  	if (flags & MAP_FIXED)
>  		return addr;
>  
> -	if (!addr)
> -		addr = randomize_range(SHLIB_BASE, 0x01000000, len);
> +	if (!addr) {
> +		addr = SHLIB_BASE;
> +		if ((current->flags & PF_RANDOMIZE) && randomize_va_space)
> +			addr = randomize_range(addr, 0x01000000, len);
> +	}
>  
>  	if (addr) {
>  		addr = PAGE_ALIGN(addr);
> @@ -1529,7 +1532,9 @@ arch_get_unmapped_exec_area(struct file *filp, unsigned long addr0,
>  			 * Up until the brk area we randomize addresses
>  			 * as much as possible:
>  			 */
> -			if (addr >= 0x01000000) {
> +			if ((current->flags & PF_RANDOMIZE) &&
> +                            randomize_va_space &&
> +                            addr >= 0x01000000) {
>  				tmp = randomize_range(0x01000000,
>  					PAGE_ALIGN(max(mm->start_brk,
>  					(unsigned long)0x08000000)), len);
Stefan Bader Jan. 22, 2010, 2:16 p.m. UTC | #3
Applied and pushed
Kees Cook Jan. 22, 2010, 4:47 p.m. UTC | #4
Hi Stefan,

On Fri, Jan 22, 2010 at 03:04:16PM +0100, Stefan Bader wrote:
> I d love to understand that foo better, maybe we can do a beer time scribble and
> talk session in Portland. But it looks ok, we got it in Lucid without flames
> erupting and it has been verified to fix the issue.

Yeah, totally.  This would be pretty valuable for me too; explaining
memory layouts and this ASLR would kind of help to solidify the ideas
in my head.

-Kees
diff mbox

Patch

diff --git a/arch/x86/kernel/process.c b/arch/x86/kernel/process.c
index 5284cd2..bf5c156 100644
--- a/arch/x86/kernel/process.c
+++ b/arch/x86/kernel/process.c
@@ -607,6 +607,16 @@  unsigned long arch_align_stack(unsigned long sp)
 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;
+	unsigned long bump = 0;
+#ifdef CONFIG_X86_32
+	/* in the case of NX emulation, shove the brk segment way out of the
+           way of the exec randomization area, since it can collide with
+	   future allocations if not. */
+	if ( (mm->get_unmapped_exec_area == arch_get_unmapped_exec_area) &&
+	     (mm->brk < 0x08000000) ) {
+		bump = (TASK_SIZE/6);
+	}
+#endif
+	return bump + (randomize_range(mm->brk, range_end, 0) ? : mm->brk);
 }
 
diff --git a/mm/mmap.c b/mm/mmap.c
index a1483c5..c6d7e53 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -1500,8 +1500,11 @@  arch_get_unmapped_exec_area(struct file *filp, unsigned long addr0,
 	if (flags & MAP_FIXED)
 		return addr;
 
-	if (!addr)
-		addr = randomize_range(SHLIB_BASE, 0x01000000, len);
+	if (!addr) {
+		addr = SHLIB_BASE;
+		if ((current->flags & PF_RANDOMIZE) && randomize_va_space)
+			addr = randomize_range(addr, 0x01000000, len);
+	}
 
 	if (addr) {
 		addr = PAGE_ALIGN(addr);
@@ -1529,7 +1532,9 @@  arch_get_unmapped_exec_area(struct file *filp, unsigned long addr0,
 			 * Up until the brk area we randomize addresses
 			 * as much as possible:
 			 */
-			if (addr >= 0x01000000) {
+			if ((current->flags & PF_RANDOMIZE) &&
+                            randomize_va_space &&
+                            addr >= 0x01000000) {
 				tmp = randomize_range(0x01000000,
 					PAGE_ALIGN(max(mm->start_brk,
 					(unsigned long)0x08000000)), len);