2008-02-06 13:45:27

by Jiri Kosina

[permalink] [raw]
Subject: [PATCH 0/2] brk and randomization fixes

Andrew,

the following two patches are the outcome of the thread "brk randomization
breaks columns" [1]. As brk randomization is already in Linus' tree, it's
definitely 2.6.25 material, so it would probably be good if you could push
them in your next bunch going to Linus, if noone has any objections.

I think it's better if they go through you than through Ingo, as they are
not x86-specific at all.

Thanks.

[1] http://lkml.org/lkml/2008/2/4/83

--
Jiri Kosina
SUSE Labs


2008-02-06 13:45:48

by Jiri Kosina

[permalink] [raw]
Subject: [PATCH 1/2] brk: check the lower bound properly

From: Jiri Kosina <[email protected]>

brk: check the lower bound properly

There is a check in sys_brk(), that tries to make sure that we do not
underflow the area that is dedicated to brk heap.

The check is however wrong, as it assumes that brk area starts immediately
after the end of the code (+bss), which is wrong for example in
environments with randomized brk start. The proper way is to check whether
the address is not below the start_brk address.

Cc: Ingo Molnar <[email protected]>
Cc: Arjan van de Ven <[email protected]>
Acked-by: Hugh Dickins <[email protected]>
Acked-by: Pavel Machek <[email protected]>
Signed-off-by: Jiri Kosina <[email protected]>

diff --git a/mm/mmap.c b/mm/mmap.c
index 8295577..1c3b48f 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -241,7 +241,7 @@ asmlinkage unsigned long sys_brk(unsigned long brk)

down_write(&mm->mmap_sem);

- if (brk < mm->end_code)
+ if (brk < mm->start_brk)
goto out;

/*

2008-02-06 13:46:08

by Jiri Kosina

[permalink] [raw]
Subject: [PATCH 2/2] ASLR: add possibility for more fine-grained tweaking

From: Jiri Kosina <[email protected]>

ASLR: add possibility for more fine-grained tweaking

Some prehistoric binaries don't like when start of brk area is located
anywhere else than just after code+bss.

This patch adds possibility to configure the default behavior of address
space randomization. In addition to that, randomize_va_space now can have
value of '2', which means full randomization including brk space.

Also, documentation of randomize_va_space is added.

Cc: Ingo Molnar <[email protected]>
Cc: Arjan van de Ven <[email protected]>
Cc: Randy Dunlap <[email protected]>
Cc: Hugh Dickins <[email protected]>
Cc: Pavel Machek <[email protected]>
Signed-off-by: Jiri Kosina <[email protected]>

diff --git a/Documentation/sysctl/kernel.txt b/Documentation/sysctl/kernel.txt
index 8984a53..91ab40d 100644
--- a/Documentation/sysctl/kernel.txt
+++ b/Documentation/sysctl/kernel.txt
@@ -41,6 +41,7 @@ show up in /proc/sys/kernel:
- pid_max
- powersave-nap [ PPC only ]
- printk
+- randomize_va_space
- real-root-dev ==> Documentation/initrd.txt
- reboot-cmd [ SPARC only ]
- rtsig-max
@@ -280,6 +281,37 @@ send before ratelimiting kicks in.

==============================================================

+randomize-va-space:
+
+This option can be used to select the type of process address
+space randomization that is used in the system, for architectures
+that support this feature.
+
+One of the following numeric values is possible:
+
+0 - [none]
+ Turn the process address space randomization off by default.
+
+1 - [conservative]
+ Conservative address space randomization makes the addresses of
+ mmap base and VDSO page randomized. This, among other things,
+ implies that shared libraries will be loaded to random addresses.
+ Also for PIE binaries, the location of code start is randomized.
+
+2 - [full]
+
+ This includes all the features that Conservative randomization
+ provides. In addition to that, also start of the brk area is
+ randomized.
+ There a few legacy applications out there (such as some ancient
+ versions of libc.so.5 from 1996) that assume that brk area starts
+ just after the end of the code+bss. These applications break when
+ start of the brk area is randomized. There are however no known
+ non-legacy applications that would be broken this way, so for most
+ systems it is safe to choose Full randomization.
+
+==============================================================
+
reboot-cmd: (Sparc only)

??? This seems to be a way to give an argument to the Sparc
diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
index 4628c42..d9f23d5 100644
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
@@ -1077,7 +1077,7 @@ static int load_elf_binary(struct linux_binprm *bprm, struct pt_regs *regs)
current->mm->start_stack = bprm->p;

#ifdef arch_randomize_brk
- if (current->flags & PF_RANDOMIZE)
+ if (current->flags & PF_RANDOMIZE && randomize_va_space == 2)
current->mm->brk = current->mm->start_brk =
arch_randomize_brk(current->mm);
#endif
diff --git a/init/Kconfig b/init/Kconfig
index 87f50df..aeb38b2 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -662,6 +662,46 @@ config SLOB

endchoice

+choice
+ prompt "Address space randomization type"
+ default RANDOMIZATION_CONSERVATIVE
+ help
+ This option allows to select the type of process address space
+ randomization that will be used by default (for those architectures
+ that support address space randomization). This option can be
+ overriden in runtime through kernel.randomize_va_space sysctl.
+
+config RANDOMIZATION_NONE
+ bool "NONE"
+ help
+ Turn the process address space randomization off by default.
+ Equivalent to sysctl kernel.randomize_va_space = 0.
+
+config RANDOMIZATION_CONSERVATIVE
+ bool "CONSERVATIVE"
+ help
+ Conservative address space randomization makes the addresses of
+ mmap base and VDSO page randomized. This, among other things,
+ implies that shared libraries will be loaded to random addresses.
+ Also for PIE binaries, the location of code start is randomized.
+ Equivalent to sysctl kernel.randomize_va_space = 1.
+
+config RANDOMIZATION_FULL
+ bool "FULL"
+ help
+ This includes all the features that Conservative randomization
+ provides. In addition to that, also start of the brk area is
+ randomized.
+ There a few legacy applications out there (such as some ancient
+ versions of libc.so.5 from 1996) that assume that the brk area
+ starts just after the end of the code+bss. These applications
+ break when start of the brk area is randomized. There are however
+ no known non-legacy applications that would be broken this way,
+ so for most systems it is safe to choose Full randomization.
+ Equivalent to sysctl kernel.randomize_va_space = 2.
+
+endchoice
+
config PROFILING
bool "Profiling support (EXPERIMENTAL)"
help
diff --git a/mm/memory.c b/mm/memory.c
index 7bb7072..e84f69a 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -82,7 +82,15 @@ void * high_memory;
EXPORT_SYMBOL(num_physpages);
EXPORT_SYMBOL(high_memory);

+#ifdef CONFIG_RANDOMIZATION_CONSERVATIVE
int randomize_va_space __read_mostly = 1;
+#else
+#ifdef CONFIG_RANDOMIZATION_FULL
+int randomize_va_space __read_mostly = 2;
+#else
+int randomize_va_space __read_mostly = 0;
+#endif
+#endif

static int __init disable_randmaps(char *s)
{

2008-02-06 13:51:20

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH 2/2] ASLR: add possibility for more fine-grained tweaking


* Jiri Kosina <[email protected]> wrote:

> ASLR: add possibility for more fine-grained tweaking
>
> Some prehistoric binaries don't like when start of brk area is located
> anywhere else than just after code+bss.
>
> This patch adds possibility to configure the default behavior of
> address space randomization. In addition to that, randomize_va_space
> now can have value of '2', which means full randomization including
> brk space.

i've already added the patch below to x86.git.

Ingo

-------------------->
Subject: brk randomization: introduce CONFIG_COMPAT_BRK
From: Ingo Molnar <[email protected]>

based on similar patch from: Pavel Machek <[email protected]>

Introduce CONFIG_COMPAT_BRK. If disabled then the kernel is free
(but not obliged to) randomize the brk area.

Heap randomization breaks ancient binaries, so we keep COMPAT_BRK
enabled by default.

Signed-off-by: Ingo Molnar <[email protected]>
---
fs/binfmt_elf.c | 2 +-
init/Kconfig | 12 ++++++++++++
mm/memory.c | 13 ++++++++++++-
3 files changed, 25 insertions(+), 2 deletions(-)

Index: linux-x86.q/fs/binfmt_elf.c
===================================================================
--- linux-x86.q.orig/fs/binfmt_elf.c
+++ linux-x86.q/fs/binfmt_elf.c
@@ -1077,7 +1077,7 @@ static int load_elf_binary(struct linux_
current->mm->start_stack = bprm->p;

#ifdef arch_randomize_brk
- if (current->flags & PF_RANDOMIZE)
+ if ((current->flags & PF_RANDOMIZE) && (randomize_va_space > 1))
current->mm->brk = current->mm->start_brk =
arch_randomize_brk(current->mm);
#endif
Index: linux-x86.q/init/Kconfig
===================================================================
--- linux-x86.q.orig/init/Kconfig
+++ linux-x86.q/init/Kconfig
@@ -541,6 +541,18 @@ config ELF_CORE
help
Enable support for generating core dumps. Disabling saves about 4k.

+config COMPAT_BRK
+ bool "Disable heap randomization"
+ default y
+ help
+ Randomizing heap placement makes heap exploits harder, but it
+ also breaks ancient binaries (including anything libc5 based).
+ This option changes the bootup default to heap randomization
+ disabled, and can be overriden runtime by setting
+ /proc/sys/kernel/randomize_va_space to 2.
+
+ On non-ancient distros (post-2000 ones) Y is usually a safe choice.
+
config BASE_FULL
default y
bool "Enable full-sized data structures for core" if EMBEDDED
Index: linux-x86.q/mm/memory.c
===================================================================
--- linux-x86.q.orig/mm/memory.c
+++ linux-x86.q/mm/memory.c
@@ -82,7 +82,18 @@ void * high_memory;
EXPORT_SYMBOL(num_physpages);
EXPORT_SYMBOL(high_memory);

-int randomize_va_space __read_mostly = 1;
+/*
+ * Randomize the address space (stacks, mmaps, brk, etc.).
+ *
+ * ( When CONFIG_COMPAT_BRK=y we exclude brk from randomization,
+ * as ancient (libc5 based) binaries can segfault. )
+ */
+int randomize_va_space __read_mostly =
+#ifdef CONFIG_COMPAT_BRK
+ 1;
+#else
+ 2;
+#endif

static int __init disable_randmaps(char *s)
{

2008-02-06 16:26:47

by Jiri Kosina

[permalink] [raw]
Subject: [PATCH] Document randomize_va_space and CONFIG_COMPAT_BRK (was Re: [PATCH 2/2] ASLR: add possibility for more fine-grained tweaking)

On Wed, 6 Feb 2008, Ingo Molnar wrote:

> i've already added the patch below to x86.git.

OK, cool, thanks.

Still, I think that we want the Documentation/sysctl/kernel.txt part of my
patch probably. Updated patch below.


From: Jiri Kosina <[email protected]>

Document randomize_va_space and CONFIG_COMPAT_BRK

diff --git a/Documentation/sysctl/kernel.txt b/Documentation/sysctl/kernel.txt
index 8984a53..dc8801d 100644
--- a/Documentation/sysctl/kernel.txt
+++ b/Documentation/sysctl/kernel.txt
@@ -41,6 +41,7 @@ show up in /proc/sys/kernel:
- pid_max
- powersave-nap [ PPC only ]
- printk
+- randomize_va_space
- real-root-dev ==> Documentation/initrd.txt
- reboot-cmd [ SPARC only ]
- rtsig-max
@@ -280,6 +281,34 @@ send before ratelimiting kicks in.

==============================================================

+randomize-va-space:
+
+This option can be used to select the type of process address
+space randomization that is used in the system, for architectures
+that support this feature.
+
+0 - Turn the process address space randomization off by default.
+
+1 - Make the addresses of mmap base, stack and VDSO page randomized.
+ This, among other things, implies that shared libraries will be
+ loaded to random addresses. Also for PIE-linked binaries, the location
+ of code start is randomized.
+
+ With heap randomization, the situation is a little bit more
+ complicated.
+ There a few legacy applications out there (such as some ancient
+ versions of libc.so.5 from 1996) that assume that brk area starts
+ just after the end of the code+bss. These applications break when
+ start of the brk area is randomized. There are however no known
+ non-legacy applications that would be broken this way, so for most
+ systems it is safe to choose full randomization. However there is
+ a CONFIG_COMPAT_BRK option for systems with ancient and/or broken
+ binaries, that makes heap non-randomized, but keeps all other
+ parts of process address space randomized if randomize_va_space
+ sysctl is turned on.
+
+==============================================================
+
reboot-cmd: (Sparc only)

??? This seems to be a way to give an argument to the Sparc

2008-02-06 23:15:45

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH] Document randomize_va_space and CONFIG_COMPAT_BRK (was Re: [PATCH 2/2] ASLR: add possibility for more fine-grained tweaking)


* Jiri Kosina <[email protected]> wrote:

> On Wed, 6 Feb 2008, Ingo Molnar wrote:
>
> > i've already added the patch below to x86.git.
>
> OK, cool, thanks.
>
> Still, I think that we want the Documentation/sysctl/kernel.txt part
> of my patch probably. Updated patch below.

thanks, applied.

i'm wondering about the following detail: i guess on 64-bit x86 kernels
we could default to !CONFIG_COMPAT_BRK? In 1997 there was no 64-bit x86.
Maybe for compat 32-bit binaries we could keep it off, but always do it
for 64-bit binaries.

Ingo

2008-02-07 09:49:56

by Jiri Kosina

[permalink] [raw]
Subject: Re: [PATCH] Document randomize_va_space and CONFIG_COMPAT_BRK (was Re: [PATCH 2/2] ASLR: add possibility for more fine-grained tweaking)

On Thu, 7 Feb 2008, Ingo Molnar wrote:

> > Still, I think that we want the Documentation/sysctl/kernel.txt part
> > of my patch probably. Updated patch below.
> thanks, applied.

.. and I have just noticed that my patch was by mistake missing signoff.
Could you please add

Signed-off-by: Jiri Kosina <[email protected]>

to the patch in your tree? Thanks.

> i'm wondering about the following detail: i guess on 64-bit x86 kernels
> we could default to !CONFIG_COMPAT_BRK? In 1997 there was no 64-bit x86.
> Maybe for compat 32-bit binaries we could keep it off, but always do it
> for 64-bit binaries.

That actually makes sense, thanks. I will send you a patch.

Thanks,

--
Jiri Kosina
SUSE Labs

2008-02-07 10:24:18

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH 2/2] ASLR: add possibility for more fine-grained tweaking

On Wed, 6 Feb 2008, Ingo Molnar wrote:
> @@ -541,6 +541,18 @@ config ELF_CORE
> help
> Enable support for generating core dumps. Disabling saves about 4k.
>
> +config COMPAT_BRK
> + bool "Disable heap randomization"
> + default y
> + help
> + Randomizing heap placement makes heap exploits harder, but it
> + also breaks ancient binaries (including anything libc5 based).
> + This option changes the bootup default to heap randomization
> + disabled, and can be overriden runtime by setting
> + /proc/sys/kernel/randomize_va_space to 2.
> +
> + On non-ancient distros (post-2000 ones) Y is usually a safe choice.

Somehow my belly feeling tells me something is wrong with this description...

Ah, a negative option (Y -> disable). So Y is always safe.

`non-ancient distros' really means `recent distros', and if you have one,
then _N_ should be a safe choice, too?

With kind regards,

Geert Uytterhoeven
Software Architect

Sony Network and Software Technology Center Europe
The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium

Phone: +32 (0)2 700 8453
Fax: +32 (0)2 700 8622
E-mail: [email protected]
Internet: http://www.sony-europe.com/

Sony Network and Software Technology Center Europe
A division of Sony Service Centre (Europe) N.V.
Registered office: Technologielaan 7 · B-1840 Londerzeel · Belgium
VAT BE 0413.825.160 · RPR Brussels
Fortis Bank Zaventem · Swift GEBABEBB08A · IBAN BE39001382358619

2008-02-07 10:32:08

by Ismail Dönmez

[permalink] [raw]
Subject: Re: [PATCH 2/2] ASLR: add possibility for more fine-grained tweaking

At Thursday 07 February 2008 around 12:23:50 Geert Uytterhoeven wrote:
> On Wed, 6 Feb 2008, Ingo Molnar wrote:
> > @@ -541,6 +541,18 @@ config ELF_CORE
> >       help
> >         Enable support for generating core dumps. Disabling saves about
> > 4k.
> > +config COMPAT_BRK
> > +     bool "Disable heap randomization"
> > +     default y
> > +     help
> > +       Randomizing heap placement makes heap exploits harder, but it
> > +       also breaks ancient binaries (including anything libc5 based).
> > +       This option changes the bootup default to heap randomization
> > +       disabled, and can be overriden runtime by setting
> > +       /proc/sys/kernel/randomize_va_space to 2.
> > +
> > +       On non-ancient distros (post-2000 ones) Y is usually a safe
> > choice.
>
> Somehow my belly feeling tells me something is wrong with this
> description...
>
> Ah, a negative option (Y -> disable).  So Y is always safe.
>
> `non-ancient distros' really means `recent distros', and if you have one,
> then _N_ should be a safe choice, too?

This indeed looks wrong. The default should be N and the text should say "On
recent distros (post-2000 ones) N is usually a safe choice".

Regards,
ismail

--
Never learn by your mistakes, if you do you may never dare to try again.

2008-02-07 10:33:46

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH 2/2] ASLR: add possibility for more fine-grained tweaking


* Geert Uytterhoeven <[email protected]> wrote:

> On Wed, 6 Feb 2008, Ingo Molnar wrote:
> > @@ -541,6 +541,18 @@ config ELF_CORE
> > help
> > Enable support for generating core dumps. Disabling saves about 4k.
> >
> > +config COMPAT_BRK
> > + bool "Disable heap randomization"
> > + default y
> > + help
> > + Randomizing heap placement makes heap exploits harder, but it
> > + also breaks ancient binaries (including anything libc5 based).
> > + This option changes the bootup default to heap randomization
> > + disabled, and can be overriden runtime by setting
> > + /proc/sys/kernel/randomize_va_space to 2.
> > +
> > + On non-ancient distros (post-2000 ones) Y is usually a safe choice.
>
> Somehow my belly feeling tells me something is wrong with this description...
>
> Ah, a negative option (Y -> disable). So Y is always safe.
>
> `non-ancient distros' really means `recent distros', and if you have
> one, then _N_ should be a safe choice, too?

yeah, you are right :-) I'll fix this.

btw., "non-ancient distros" does not just mean "recent distros", it
really means "just about any distro you picked up in the past 10 years".
You'd have to go out on a limb to find something historic (or keep
copying /lib/libc5 binaries to new distros like Pavel did) to still have
this particular libc5 assumption/breakage. [ Or at least so i hope =B-)]

Ingo

2008-02-07 10:43:53

by Geert Uytterhoeven

[permalink] [raw]
Subject: Re: [PATCH 2/2] ASLR: add possibility for more fine-grained tweaking

On Thu, 7 Feb 2008, Ingo Molnar wrote:
> * Geert Uytterhoeven <[email protected]> wrote:
> > On Wed, 6 Feb 2008, Ingo Molnar wrote:
> > > @@ -541,6 +541,18 @@ config ELF_CORE
> > > help
> > > Enable support for generating core dumps. Disabling saves about 4k.
> > >
> > > +config COMPAT_BRK
> > > + bool "Disable heap randomization"
> > > + default y
> > > + help
> > > + Randomizing heap placement makes heap exploits harder, but it
> > > + also breaks ancient binaries (including anything libc5 based).
> > > + This option changes the bootup default to heap randomization
> > > + disabled, and can be overriden runtime by setting
> > > + /proc/sys/kernel/randomize_va_space to 2.
> > > +
> > > + On non-ancient distros (post-2000 ones) Y is usually a safe choice.
> >
> > Somehow my belly feeling tells me something is wrong with this description...
> >
> > Ah, a negative option (Y -> disable). So Y is always safe.
> >
> > `non-ancient distros' really means `recent distros', and if you have
> > one, then _N_ should be a safe choice, too?
>
> yeah, you are right :-) I'll fix this.
>
> btw., "non-ancient distros" does not just mean "recent distros", it
> really means "just about any distro you picked up in the past 10 years".
> You'd have to go out on a limb to find something historic (or keep
> copying /lib/libc5 binaries to new distros like Pavel did) to still have
> this particular libc5 assumption/breakage. [ Or at least so i hope =B-)]

Bummer, I guess my good old m68k recovery ramdisk is affected ;-)

With kind regards,

Geert Uytterhoeven
Software Architect

Sony Network and Software Technology Center Europe
The Corporate Village · Da Vincilaan 7-D1 · B-1935 Zaventem · Belgium

Phone: +32 (0)2 700 8453
Fax: +32 (0)2 700 8622
E-mail: [email protected]
Internet: http://www.sony-europe.com/

Sony Network and Software Technology Center Europe
A division of Sony Service Centre (Europe) N.V.
Registered office: Technologielaan 7 · B-1840 Londerzeel · Belgium
VAT BE 0413.825.160 · RPR Brussels
Fortis Bank Zaventem · Swift GEBABEBB08A · IBAN BE39001382358619

2008-02-07 14:30:56

by Jiri Kosina

[permalink] [raw]
Subject: Re: [PATCH] Document randomize_va_space and CONFIG_COMPAT_BRK (was Re: [PATCH 2/2] ASLR: add possibility for more fine-grained tweaking)

On Thu, 7 Feb 2008, Ingo Molnar wrote:

> i'm wondering about the following detail: i guess on 64-bit x86 kernels
> we could default to !CONFIG_COMPAT_BRK? In 1997 there was no 64-bit x86.
> Maybe for compat 32-bit binaries we could keep it off, but always do it
> for 64-bit binaries.

So what do you think is proper behavior in situation when
CONFIG_COMPAT_BRK=N on 64bit kernel, and 32bit-binary is loaded in 32bit
emulation?

We can either leave the brk as-is, but that is in contradiction to user
explictly specifying CONFIG_COMPAT_BRK=N. Is this what you propose?

Or we can randomize brk start in such situation, but that is the behavior
we currently automatically have due to CONFIG_COMPAT_BRK=N, so no change
is needed.

Thanks,

--
Jiri Kosina
SUSE Labs

2008-02-07 15:02:35

by Ingo Molnar

[permalink] [raw]
Subject: Re: [PATCH] Document randomize_va_space and CONFIG_COMPAT_BRK (was Re: [PATCH 2/2] ASLR: add possibility for more fine-grained tweaking)


* Jiri Kosina <[email protected]> wrote:

> On Thu, 7 Feb 2008, Ingo Molnar wrote:
>
> > i'm wondering about the following detail: i guess on 64-bit x86
> > kernels we could default to !CONFIG_COMPAT_BRK? In 1997 there was no
> > 64-bit x86. Maybe for compat 32-bit binaries we could keep it off,
> > but always do it for 64-bit binaries.
>
> So what do you think is proper behavior in situation when
> CONFIG_COMPAT_BRK=N on 64bit kernel, and 32bit-binary is loaded in
> 32bit emulation?
>
> We can either leave the brk as-is, but that is in contradiction to
> user explictly specifying CONFIG_COMPAT_BRK=N. Is this what you
> propose?
>
> Or we can randomize brk start in such situation, but that is the
> behavior we currently automatically have due to CONFIG_COMPAT_BRK=N,
> so no change is needed.

thinking about it ... i think we should just keep this simple, and when
COMPAT_BRK=y then we disable brk randomization globally. If !COMPAT_BRK
then we do brk randomization globally as well. (and that is probably
what users want the sysctl to do anyway - users wont necessarily know
whether the app breakage they want to solve is due to 32-bit or 64-bit.)

Ingo