2008-02-04 12:28:34

by Pavel Machek

[permalink] [raw]
Subject: brk randomization breaks columns

Hi!

Columns is very popular game of year about 1993, and brk randomization
breaks it. (Along with my boot, but who cares about boot when game is
broken?)

echo 1 > /proc/sys/kernel/randomize_va_space

breaks columns

echo 0 > /proc/sys/kernel/randomize_va_space

fixes them.

root@amd:~# ls -al `which columns-bin`
-rwxr-xr-x 1 root root 100515 Aug 7 1997 /usr/local/bin/columns-bin*
root@amd:~# ldd `which columns-bin`
libc.so.5 => /lib/libc.so.5 (0xb7e22000)
root@amd:~#

pavel@amd:~$ strace columns-bin
execve("/usr/local/bin/columns-bin", ["columns-bin"], [/* 31 vars */])
= 0
old_mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS,
-1, 0) = 0xb7f78000
mprotect(0xb7f79000, 21406, PROT_READ|PROT_WRITE|PROT_EXEC) = 0
mprotect(0x8048000, 31345, PROT_READ|PROT_WRITE|PROT_EXEC) = 0
stat("/etc/ld.so.cache", {st_mode=S_IFREG|0644, st_size=106939, ...})
= 0
open("/etc/ld.so.cache", O_RDONLY) = 3
old_mmap(NULL, 106939, PROT_READ, MAP_SHARED, 3, 0) = 0xb7f5d000
close(3) = 0
stat("/etc/ld.so.preload", 0xbf87f348) = -1 ENOENT (No such file or
directory)
open("/home/pavel/lib/libc.so.5", O_RDONLY) = -1 ENOENT (No such file
or directory)
open("/lib/libc.so.5", O_RDONLY) = 3
read(3, "\177ELF\1\1\1\0\0\0\0\0\0\0\0\0\3\0\3\0\1\0\0\0\240\32"...,
4096) = 4096
old_mmap(NULL, 786432, PROT_NONE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) =
0xb7e9d000
old_mmap(0xb7e9d000, 552787, PROT_READ|PROT_EXEC,
MAP_PRIVATE|MAP_FIXED, 3, 0) = 0xb7e9d000
old_mmap(0xb7f24000, 21848, PROT_READ|PROT_WRITE,
MAP_PRIVATE|MAP_FIXED, 3, 0x86000) = 0xb7f24000
old_mmap(0xb7f2a000, 204908, PROT_READ|PROT_WRITE,
MAP_PRIVATE|MAP_FIXED|MAP_ANONYMOUS, -1, 0) = 0xb7f2a000
close(3) = 0
mprotect(0xb7e9d000, 552787, PROT_READ|PROT_WRITE|PROT_EXEC) = 0
munmap(0xb7f5d000, 106939) = 0
mprotect(0x8048000, 31345, PROT_READ|PROT_EXEC) = 0
mprotect(0xb7e9d000, 552787, PROT_READ|PROT_EXEC) = 0
mprotect(0xb7f79000, 21406, PROT_READ|PROT_EXEC) = 0
personality(PER_LINUX) = 4194304
geteuid() = 1000
getuid() = 1000
getgid() = 1002
getegid() = 1002
brk(0x8054098) = 0x8054098
brk(0x8055000) = 0x8055000
--- SIGSEGV (Segmentation fault) @ 0 (0) ---
+++ killed by SIGSEGV (core dumped) +++
Process 1517 detached
pavel@amd:~$

columns die due to

Feb 4 12:29:32 amd kernel: columns-bin[4535]: segfault at 8052000 ip b7f08a9a sp bfb79628 error 6 in
libc.so.5.4.33[b7e99000+87000]

Just before death,

root@amd:~# cat /proc/4537/maps
08048000-08050000 r-xp 00000000 08:04 246209 /usr/local/bin/columns-bin
08050000-08051000 rwxp 00007000 08:04 246209 /usr/local/bin/columns-bin
08051000-08052000 rwxp 08051000 00:00 0
b7f00000-b7f87000 r-xp 00000000 08:04 373330 /lib/libc.so.5.4.33
b7f87000-b7f8d000 rwxp 00086000 08:04 373330 /lib/libc.so.5.4.33
b7f8d000-b7fc0000 rwxp b7f8d000 00:00 0
b7fdb000-b7fdc000 rwxp b7fdb000 00:00 0
b7fdc000-b7fe2000 r-xp 00000000 08:04 373339 /lib/ld-linux.so.1.9.11
b7fe2000-b7fe3000 rwxp 00005000 08:04 373339 /lib/ld-linux.so.1.9.11
bface000-bfae3000 rwxp bffeb000 00:00 0 [stack]
ffffe000-fffff000 r-xp 00000000 00:00 0 [vdso]
root@amd:~#

...which is strange. Columns asked for brk, but kernel assigned it no
heap. No wonder columns are crashing.

(gdb) bt
#0 0xb7f6fa60 in memset () from /lib/libc.so.5
#1 0xb7f7b4a3 in initialize () from /lib/libc.so.5
#2 0x00000024 in ?? ()
#3 0x00000000 in ?? ()
(gdb)
(gdb) disassemble
Dump of assembler code for function memset:
0xb7f6fa60 <memset+0>: push %ebp
0xb7f6fa61 <memset+1>: push %edi
0xb7f6fa62 <memset+2>: push %esi
0xb7f6fa63 <memset+3>: mov 0x10(%esp),%ebp
0xb7f6fa67 <memset+7>: mov 0x18(%esp),%esi
0xb7f6fa6b <memset+11>: mov %ebp,%edi
0xb7f6fa6d <memset+13>: movzbl 0x14(%esp),%eax
0xb7f6fa72 <memset+18>: cld
0xb7f6fa73 <memset+19>: cmp $0xb,%esi
0xb7f6fa76 <memset+22>: jbe 0xb7f6fa9f <memset+63>
0xb7f6fa78 <memset+24>: mov %eax,%edx
0xb7f6fa7a <memset+26>: shl $0x8,%edx
0xb7f6fa7d <memset+29>: or %edx,%eax
0xb7f6fa7f <memset+31>: mov %eax,%edx
0xb7f6fa81 <memset+33>: shl $0x10,%edx
0xb7f6fa84 <memset+36>: or %edx,%eax
0xb7f6fa86 <memset+38>: mov %ebp,%edx
0xb7f6fa88 <memset+40>: neg %edx
0xb7f6fa8a <memset+42>: and $0x3,%edx
0xb7f6fa8d <memset+45>: sub %edx,%esi
0xb7f6fa8f <memset+47>: mov %edx,%ecx
0xb7f6fa91 <memset+49>: rep stos %al,%es:(%edi)
0xb7f6fa93 <memset+51>: mov %esi,%edx
0xb7f6fa95 <memset+53>: shr $0x2,%edx
0xb7f6fa98 <memset+56>: mov %edx,%ecx
0xb7f6fa9a <memset+58>: rep stos %eax,%es:(%edi)
0xb7f6fa9c <memset+60>: and $0x3,%esi
0xb7f6fa9f <memset+63>: mov %esi,%ecx
0xb7f6faa1 <memset+65>: rep stos %al,%es:(%edi)
0xb7f6faa3 <memset+67>: mov %ebp,%eax
0xb7f6faa5 <memset+69>: pop %esi
0xb7f6faa6 <memset+70>: pop %edi
0xb7f6faa7 <memset+71>: pop %ebp
0xb7f6faa8 <memset+72>: ret
End of assembler dump.
(gdb)
(gdb) i r
eax 0x3000 12288
ecx 0x8055000 134565888
edx 0xb7f8ac68 -1208439704
ebx 0xb7f8bb08 -1208435960
esp 0xbfae1db4 0xbfae1db4
ebp 0xb7fbf058 0xb7fbf058
esi 0xf68 3944
edi 0x8052000 134553600
eip 0xb7f6fa60 0xb7f6fa60 <memset>
eflags 0x282 [ SF IF ]
cs 0x73 115
ss 0x7b 123
ds 0x7b 123
es 0x7b 123
fs 0x0 0
gs 0x0 0
(gdb)


Hmm, code in binfmt_elf is really strange.

elf_bss += load_bias;
elf_brk += load_bias;
start_code += load_bias;
end_code += load_bias;
start_data += load_bias;
end_data += load_bias;

/* Calling set_brk effectively mmaps the pages that we need
* for the bss and break sections. We must do this before
* mapping in the interpreter, to make sure it doesn't wind
* up getting placed where the bss needs to go.
*/
retval = set_brk(elf_bss, elf_brk);

... so we allocate non-randoimzed brk, but later we just overwrite bss
variable with new, shiner and better randomized value... without
unmapping the old one... The code in binfmt_elf.c is really a mess.

Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html


2008-02-04 13:02:18

by Ingo Molnar

[permalink] [raw]
Subject: Re: brk randomization breaks columns


* Pavel Machek <[email protected]> wrote:

> Hmm, code in binfmt_elf is really strange.
>
> elf_bss += load_bias;
> elf_brk += load_bias;
> start_code += load_bias;
> end_code += load_bias;
> start_data += load_bias;
> end_data += load_bias;
>
> /* Calling set_brk effectively mmaps the pages that we need
> * for the bss and break sections. We must do this before
> * mapping in the interpreter, to make sure it doesn't wind
> * up getting placed where the bss needs to go.
> */
> retval = set_brk(elf_bss, elf_brk);
>
> ... so we allocate non-randoimzed brk, but later we just overwrite bss
> variable with new, shiner and better randomized value... without
> unmapping the old one... The code in binfmt_elf.c is really a mess.

hm, so it seems that it isnt even the randomization that causes the
problem - but somehow the randomization code itself is broken, right?

Would you be interested in figuring out how to unbreak this? [if not,
could you send me the binary?]

Ingo

2008-02-04 13:28:47

by Pavel Machek

[permalink] [raw]
Subject: Re: brk randomization breaks columns

Hi!

> > Hmm, code in binfmt_elf is really strange.
> >
> > elf_bss += load_bias;
> > elf_brk += load_bias;
> > start_code += load_bias;
> > end_code += load_bias;
> > start_data += load_bias;
> > end_data += load_bias;
> >
> > /* Calling set_brk effectively mmaps the pages that we need
> > * for the bss and break sections. We must do this before
> > * mapping in the interpreter, to make sure it doesn't wind
> > * up getting placed where the bss needs to go.
> > */
> > retval = set_brk(elf_bss, elf_brk);
> >
> > ... so we allocate non-randoimzed brk, but later we just overwrite bss
> > variable with new, shiner and better randomized value... without
> > unmapping the old one... The code in binfmt_elf.c is really a mess.
>
> hm, so it seems that it isnt even the randomization that causes the
> problem - but somehow the randomization code itself is broken, right?
>
> Would you be interested in figuring out how to unbreak this? [if not,
> could you send me the binary?]

Not sure this helps... If I only randomize _end_ of heap, it still
works. If I try to randomize beggining of heap, too, it will not even
start recent binaries :-(.
Pavel

diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
index 18ed6dd..9afc58f 100644
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
@@ -980,6 +983,16 @@ #endif
}

loc->elf_ex.e_entry += load_bias;
+
+
+ printk("%d: %x\n", current->pid, elf_brk);
+
+ extern unsigned long randomize_range(unsigned long start, unsigned long end, unsigned long len);
+
+ long random = randomize_range(0, 0x200000, 0);
+ elf_brk += random;
+// elf_bss += random;
+
elf_bss += load_bias;
elf_brk += load_bias;
start_code += load_bias;
@@ -1076,12 +1093,6 @@ #endif /* ARCH_HAS_SETUP_ADDITIONAL_PAGE
current->mm->end_data = end_data;
current->mm->start_stack = bprm->p;

-#ifdef arch_randomize_brk
- if (current->flags & PF_RANDOMIZE)
- current->mm->brk = current->mm->start_brk =
- arch_randomize_brk(current->mm);
-#endif
-
if (current->personality & MMAP_PAGE_ZERO) {
/* Why this, you ask??? Well SVr4 maps page 0 as read-only,
and some applications "depend" upon this behavior.

--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

2008-02-04 14:34:23

by Jiri Kosina

[permalink] [raw]
Subject: Re: brk randomization breaks columns

On Mon, 4 Feb 2008, Ingo Molnar wrote:

> hm, so it seems that it isnt even the randomization that causes the
> problem - but somehow the randomization code itself is broken, right?
> Would you be interested in figuring out how to unbreak this? [if not,
> could you send me the binary?]

I still don't seem to fully understand what is happening here -- aparently
this is triggerable only with old programs linked against libc.so.5, and I
am not able to trigger it with my trivial program when I link it against
old libc.so.5, which just basically does brk() and checks whether
/proc/<pid>/maps are OK. Seems to me that (at least certain versions) of
libc.so.5 (wrongly) assume that end of the bss is the start of the heap,
but I will try to investigate it more.

Ingo, the code in commit c1d171a0029 is IMHO funcionalli identical to
your exec-shield in fedora kernels, so even libc.so.5-linked binaries on
any Fedora distro should trigger this bug ... (or you can use the binary
that Pavel supplied). Haven't tried this yet.

Thanks,

--
Jiri Kosina

2008-02-04 14:55:29

by Jiri Kosina

[permalink] [raw]
Subject: Re: brk randomization breaks columns

On Mon, 4 Feb 2008, Pavel Machek wrote:

> Not sure this helps... If I only randomize _end_ of heap, it still
> works. If I try to randomize beggining of heap, too, it will not even
> start recent binaries :-(.

I don't uderstand this, sorry. Ehen the mapping for the new process is
being established during loading of the binary, the beginning and the end
of the heap are the same ...

--
Jiri Kosina

2008-02-04 16:13:09

by Jiri Kosina

[permalink] [raw]
Subject: Re: brk randomization breaks columns

On Mon, 4 Feb 2008, Jiri Kosina wrote:

> I still don't seem to fully understand what is happening here --
> aparently this is triggerable only with old programs linked against
> libc.so.5, and I am not able to trigger it with my trivial program when
> I link it against old libc.so.5, which just basically does brk() and
> checks whether /proc/<pid>/maps are OK. Seems to me that (at least
> certain versions) of libc.so.5 (wrongly) assume that end of the bss is
> the start of the heap, but I will try to investigate it more.

And I really think that (at least Pavel's version of) libc.so.5 is making
some strange assumptions about memory layout of the process, which could
also explain the other failures.

Pavel, could you please link the source code below against libc.so.5 with
brk randomization turned on, and show the output, and if it segfaults,
send the backtrace at the time of the crash? I suspect that it will crash
somewhere in exit path when calling .dtor functions, as it will try to
perform some cleanup in unmapped area. Thanks.

#include <unistd.h>
#include <stdio.h>
#include <stdlib.h>

int main()
{
int *curr_brk = sbrk(0);
char command[1024];

sprintf(command, "/bin/cat /proc/%d/maps", getpid());

printf("brk set to %p\n", curr_brk);
system(command);

brk(curr_brk + 0x00001000);
curr_brk = sbrk(0);

printf("brk set to %p\n", curr_brk);
system(command);
exit(0);
}

--
Jiri Kosina
SUSE Labs

2008-02-04 20:25:28

by Pavel Machek

[permalink] [raw]
Subject: Re: brk randomization breaks columns

Hi!

> > Not sure this helps... If I only randomize _end_ of heap, it still
> > works. If I try to randomize beggining of heap, too, it will not even
> > start recent binaries :-(.
>
> I don't uderstand this, sorry. Ehen the mapping for the new process is
> being established during loading of the binary, the beginning and the end
> of the heap are the same ...

Okay, I was assuming this is mapping heap..

/* Calling set_brk effectively mmaps the pages that we need
* for the bss and break sections. We must do this before
* mapping in the interpreter, to make sure it doesn't wind
* up getting placed where the bss needs to go.
*/
retval = set_brk(elf_bss, elf_brk);

If I add

+ elf_brk += random;

before set_brk() (and revert the brk randomization), it works,
including columns.

If I add

+// elf_bss += random;

before set_brk(), I break everything.
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

2008-02-04 20:31:03

by Pavel Machek

[permalink] [raw]
Subject: Re: brk randomization breaks columns

On Mon 2008-02-04 17:12:43, Jiri Kosina wrote:
> On Mon, 4 Feb 2008, Jiri Kosina wrote:
>
> > I still don't seem to fully understand what is happening here --
> > aparently this is triggerable only with old programs linked against
> > libc.so.5, and I am not able to trigger it with my trivial program when
> > I link it against old libc.so.5, which just basically does brk() and
> > checks whether /proc/<pid>/maps are OK. Seems to me that (at least
> > certain versions) of libc.so.5 (wrongly) assume that end of the bss is
> > the start of the heap, but I will try to investigate it more.
>
> And I really think that (at least Pavel's version of) libc.so.5 is making
> some strange assumptions about memory layout of the process, which could
> also explain the other failures.
>
> Pavel, could you please link the source code below against libc.so.5 with
> brk randomization turned on, and show the output, and if it segfaults,
> send the backtrace at the time of the crash? I suspect that it will crash
> somewhere in exit path when calling .dtor functions, as it will try to
> perform some cleanup in unmapped area. Thanks.
>
> #include <unistd.h>
> #include <stdio.h>
> #include <stdlib.h>
>
> int main()
> {
> int *curr_brk = sbrk(0);
> char command[1024];
>
> sprintf(command, "/bin/cat /proc/%d/maps", getpid());
>
> printf("brk set to %p\n", curr_brk);
> system(command);
>
> brk(curr_brk + 0x00001000);
> curr_brk = sbrk(0);
>
> printf("brk set to %p\n", curr_brk);
> system(command);
> exit(0);
> }

This one seems to work, even with randomization:

guest@amd:~$ gcc test2.c -nostdlib /lib/libc.so.5.4.33 -o test2
/usr/bin/ld: warning: cannot find entry symbol _start; defaulting to
0000000008048340
guest@amd:~$ ./test2
brk set to 0x8b1b000
08048000-08049000 r-xp 00000000 08:04 423985 /home/guest/test2
08049000-0804a000 rwxp 00000000 08:04 423985 /home/guest/test2
b7eb7000-b7eb8000 rwxp b7eb7000 00:00 0
b7eb8000-b7f3f000 r-xp 00000000 08:04 373330 /lib/libc.so.5.4.33
b7f3f000-b7f45000 rwxp 00086000 08:04 373330 /lib/libc.so.5.4.33
b7f45000-b7f78000 rwxp b7f45000 00:00 0
b7f92000-b7f95000 rwxp b7f92000 00:00 0
b7f95000-b7fb1000 r-xp 00000000 08:04 194956 /lib/ld-2.7.so
b7fb1000-b7fb3000 rwxp 0001b000 08:04 194956 /lib/ld-2.7.so
bf89e000-bf8b2000 rwxp bffeb000 00:00 0 [stack]
bf8b2000-bf8b3000 rw-p bffff000 00:00 0
ffffe000-fffff000 r-xp 00000000 00:00 0 [vdso]
brk set to 0x8b1f000
08048000-08049000 r-xp 00000000 08:04 423985 /home/guest/test2
08049000-0804a000 rwxp 00000000 08:04 423985 /home/guest/test2
08b1b000-08b1f000 rwxp 08b1b000 00:00 0 [heap]
b7eb7000-b7eb8000 rwxp b7eb7000 00:00 0
b7eb8000-b7f3f000 r-xp 00000000 08:04 373330 /lib/libc.so.5.4.33
b7f3f000-b7f45000 rwxp 00086000 08:04 373330 /lib/libc.so.5.4.33
b7f45000-b7f78000 rwxp b7f45000 00:00 0
b7f92000-b7f95000 rwxp b7f92000 00:00 0
b7f95000-b7fb1000 r-xp 00000000 08:04 194956 /lib/ld-2.7.so
b7fb1000-b7fb3000 rwxp 0001b000 08:04 194956 /lib/ld-2.7.so
bf89e000-bf8b2000 rwxp bffeb000 00:00 0 [stack]
bf8b2000-bf8b3000 rw-p bffff000 00:00 0
ffffe000-fffff000 r-xp 00000000 00:00 0 [vdso]
guest@amd:~$

But if I compile it on my system, test2 uses ld-linux.so.2, not
ld-linux.so.1. columns use ld-linux.so.1, but I can't compile binary
like that with my toolchain.

Pavel

--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

2008-02-05 01:58:20

by Jiri Kosina

[permalink] [raw]
Subject: Re: brk randomization breaks columns

[ some CCs added ]

On Mon, 4 Feb 2008, Pavel Machek wrote:

> pavel@amd:~$ strace columns-bin
> execve("/usr/local/bin/columns-bin", ["columns-bin"], [/* 31 vars */])
> = 0
> old_mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS,
> -1, 0) = 0xb7f78000
> mprotect(0xb7f79000, 21406, PROT_READ|PROT_WRITE|PROT_EXEC) = 0
> mprotect(0x8048000, 31345, PROT_READ|PROT_WRITE|PROT_EXEC) = 0
> stat("/etc/ld.so.cache", {st_mode=S_IFREG|0644, st_size=106939, ...})
> = 0
> open("/etc/ld.so.cache", O_RDONLY) = 3
> old_mmap(NULL, 106939, PROT_READ, MAP_SHARED, 3, 0) = 0xb7f5d000
> close(3) = 0
> stat("/etc/ld.so.preload", 0xbf87f348) = -1 ENOENT (No such file or
> directory)
> open("/home/pavel/lib/libc.so.5", O_RDONLY) = -1 ENOENT (No such file
> or directory)
> open("/lib/libc.so.5", O_RDONLY) = 3
> read(3, "\177ELF\1\1\1\0\0\0\0\0\0\0\0\0\3\0\3\0\1\0\0\0\240\32"...,
> 4096) = 4096
> old_mmap(NULL, 786432, PROT_NONE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) =
> 0xb7e9d000
> old_mmap(0xb7e9d000, 552787, PROT_READ|PROT_EXEC,
> MAP_PRIVATE|MAP_FIXED, 3, 0) = 0xb7e9d000
> old_mmap(0xb7f24000, 21848, PROT_READ|PROT_WRITE,
> MAP_PRIVATE|MAP_FIXED, 3, 0x86000) = 0xb7f24000
> old_mmap(0xb7f2a000, 204908, PROT_READ|PROT_WRITE,
> MAP_PRIVATE|MAP_FIXED|MAP_ANONYMOUS, -1, 0) = 0xb7f2a000
> close(3) = 0
> mprotect(0xb7e9d000, 552787, PROT_READ|PROT_WRITE|PROT_EXEC) = 0
> munmap(0xb7f5d000, 106939) = 0
> mprotect(0x8048000, 31345, PROT_READ|PROT_EXEC) = 0
> mprotect(0xb7e9d000, 552787, PROT_READ|PROT_EXEC) = 0
> mprotect(0xb7f79000, 21406, PROT_READ|PROT_EXEC) = 0
> personality(PER_LINUX) = 4194304
> geteuid() = 1000
> getuid() = 1000
> getgid() = 1002
> getegid() = 1002
> brk(0x8054098) = 0x8054098
> brk(0x8055000) = 0x8055000
> --- SIGSEGV (Segmentation fault) @ 0 (0) ---
> +++ killed by SIGSEGV (core dumped) +++
> Process 1517 detached
> pavel@amd:~$
> columns die due to
> Feb 4 12:29:32 amd kernel: columns-bin[4535]: segfault at 8052000 ip b7f08a9a sp bfb79628 error 6 in
> libc.so.5.4.33[b7e99000+87000]
> Just before death,
> root@amd:~# cat /proc/4537/maps
> 08048000-08050000 r-xp 00000000 08:04 246209 /usr/local/bin/columns-bin
> 08050000-08051000 rwxp 00007000 08:04 246209 /usr/local/bin/columns-bin
> 08051000-08052000 rwxp 08051000 00:00 0
> b7f00000-b7f87000 r-xp 00000000 08:04 373330 /lib/libc.so.5.4.33
> b7f87000-b7f8d000 rwxp 00086000 08:04 373330 /lib/libc.so.5.4.33
> b7f8d000-b7fc0000 rwxp b7f8d000 00:00 0
> b7fdb000-b7fdc000 rwxp b7fdb000 00:00 0
> b7fdc000-b7fe2000 r-xp 00000000 08:04 373339 /lib/ld-linux.so.1.9.11
> b7fe2000-b7fe3000 rwxp 00005000 08:04 373339 /lib/ld-linux.so.1.9.11
> bface000-bfae3000 rwxp bffeb000 00:00 0 [stack]
> ffffe000-fffff000 r-xp 00000000 00:00 0 [vdso]
> root@amd:~#

Actually, this clearly shows that either prehistoric libc.so.5 or the
program itself are broken.

- as you can easily see by repeated invocation of your program, the
arguments to brk() are always the same, no matter to what offset the brk
start gets randomized.

- i.e. the arguments passed to brk() strace shows clearly indicate that
the binary (or library) is assuming that brk starts in the very next
page after code+bss (i.e. at the page following 0x08052000). That is wrong.
The program then accessess unmapped memory, which causes segfault.

> ...which is strange. Columns asked for brk, but kernel assigned it no
> heap. No wonder columns are crashing.

Now, you are right that the return value from brk() is bogus in these
cases. The patch below should make it behave, as you can easily check with
strace, right? Does anyone have any comments regarding this patch please?

Still, it will probably not fix your particular program crashes, just
because it will always assume that brk starts immediately after the end of
the bss, which is plain wrong and has never been assured. Could you please
check whether there is any compat-* package available for you
distribution, that upgrades libc.so.5 to any fixed version?

Thanks.


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.

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-05 11:06:26

by Pavel Machek

[permalink] [raw]
Subject: [regression] Re: brk randomization breaks columns


[some more CCs added]

> > Feb 4 12:29:32 amd kernel: columns-bin[4535]: segfault at 8052000 ip b7f08a9a sp bfb79628 error 6 in
> > libc.so.5.4.33[b7e99000+87000]
> > Just before death,
> > root@amd:~# cat /proc/4537/maps
> > 08048000-08050000 r-xp 00000000 08:04 246209 /usr/local/bin/columns-bin
> > 08050000-08051000 rwxp 00007000 08:04 246209 /usr/local/bin/columns-bin
> > 08051000-08052000 rwxp 08051000 00:00 0
> > b7f00000-b7f87000 r-xp 00000000 08:04 373330 /lib/libc.so.5.4.33
> > b7f87000-b7f8d000 rwxp 00086000 08:04 373330 /lib/libc.so.5.4.33
> > b7f8d000-b7fc0000 rwxp b7f8d000 00:00 0
> > b7fdb000-b7fdc000 rwxp b7fdb000 00:00 0
> > b7fdc000-b7fe2000 r-xp 00000000 08:04 373339 /lib/ld-linux.so.1.9.11
> > b7fe2000-b7fe3000 rwxp 00005000 08:04 373339 /lib/ld-linux.so.1.9.11
> > bface000-bfae3000 rwxp bffeb000 00:00 0 [stack]
> > ffffe000-fffff000 r-xp 00000000 00:00 0 [vdso]
> > root@amd:~#
>
> Actually, this clearly shows that either prehistoric libc.so.5 or the
> program itself are broken.

I believe it shows clear regression in latest 2.6.25 kernel.

> - as you can easily see by repeated invocation of your program, the
> arguments to brk() are always the same, no matter to what offset the brk
> start gets randomized.

> - i.e. the arguments passed to brk() strace shows clearly indicate that
> the binary (or library) is assuming that brk starts in the very next
> page after code+bss (i.e. at the page following 0x08052000). That is wrong.
> The program then accessess unmapped memory, which causes segfault.

You say it is wrong. Manpages imply otherwise:

int brk(void *end_data_segment);
...
DESCRIPTION
brk() sets the end of the data segment to the value specified by
end_data_segment, when that value is reasonable, the system does have enough
memory and the process does not exceed its max data size (see setrlimit(2)).
...

Note it talks about data segment, not about heap, and that seems to
imply that BSS and heap are actually one area. 2.6.25 broke that.

Now, maybe you are right and heap randomization is useful, but
breaking 10year old executables is no-no. (And you should have posted
man page update, too).

There are ways around this:

1) not enable heap randomization unless app asks for it by personality
syscall

or

2) (hacky!) detect that app asks for brk() below its heap start, which
means it assumes BSS+heap are contiguous, and just map the memory there.

> > ...which is strange. Columns asked for brk, but kernel assigned it no
> > heap. No wonder columns are crashing.
>
> Now, you are right that the return value from brk() is bogus in these
> cases. The patch below should make it behave, as you can easily check with
> strace, right? Does anyone have any comments regarding this patch
> please?

Yep, will try, sound like a good idea.

> Still, it will probably not fix your particular program crashes, just
> because it will always assume that brk starts immediately after the end of
> the bss, which is plain wrong and has never been assured. Could you please

Can you quote docs that tells me it is plain wrong? It was plain right
in BSD unix, and in all linuxes up to 2.6.25-git. My man pages still
imply it is right.

Hmm, and even the kernel code implies it is right.

Pavel
> 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;
>

You see? This assumed end_code == start_brk ;-).

--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

2008-02-05 12:51:01

by Jiri Kosina

[permalink] [raw]
Subject: Re: [regression] Re: brk randomization breaks columns

On Tue, 5 Feb 2008, Pavel Machek wrote:

> > Actually, this clearly shows that either prehistoric libc.so.5 or the
> > program itself are broken.
> I believe it shows clear regression in latest 2.6.25 kernel.

I am still not completely sure. It might be a regression, but it also
might just trigger the bug in ancient version in libc.so.5 which might be
fixed in some later version -- are you able to verify that?

> You say it is wrong. Manpages imply otherwise:
>
> int brk(void *end_data_segment);
> ...
> DESCRIPTION
> brk() sets the end of the data segment to the value specified by
> end_data_segment, when that value is reasonable, the system does have enough
> memory and the process does not exceed its max data size (see setrlimit(2)).
> Note it talks about data segment, not about heap, and that seems to
> imply that BSS and heap are actually one area. 2.6.25 broke that.

Single Unix Specification talks only about manipulating the break section,
see http://opengroup.org/onlinepubs/007908775/xsh/brk.html

> Now, maybe you are right and heap randomization is useful, but breaking
> 10year old executables is no-no.

Even if the bug is in 10year old library which might have been fixed by a
later update? (I don't know how to verify this, libc.so.5 is so ancient
that it's difficult to find anything about that).

> 1) not enable heap randomization unless app asks for it by personality
> syscall

That (beyond other things) doesn't fit into the whole randomization
picture, as all other aspect of memory space randomization are dependent
only on 'randomize_va_space' and nothing else ... adding something special
just for brk seems to be messy.

> 2) (hacky!) detect that app asks for brk() below its heap start, which
> means it assumes BSS+heap are contiguous, and just map the memory there.

That's really ugly :)

> > Still, it will probably not fix your particular program crashes, just
> > because it will always assume that brk starts immediately after the end of
> > the bss, which is plain wrong and has never been assured. Could you please
> Can you quote docs that tells me it is plain wrong?

See the Single Unix Specification. It doesn't seem to allow you to assume
*anything* about start_brk location, seems to me.

Now, I am perfectly fine with reverting that patch for 2.6.25 unless
someone is able to come up with something clever. It is quite unfortunate
though, that we possibly give up quite reasonable security measure just
because 10-year-old libc assumes something that it possibly isn't allowed
to. Arjan, I know you have been working on brk randomization previously,
do you have any input here?

Thanks,

--
Jiri Kosina
SUSE Labs

2008-02-05 12:55:23

by Ingo Molnar

[permalink] [raw]
Subject: Re: [regression] Re: brk randomization breaks columns


* Jiri Kosina <[email protected]> wrote:

> On Tue, 5 Feb 2008, Pavel Machek wrote:
>
> > > Actually, this clearly shows that either prehistoric libc.so.5 or the
> > > program itself are broken.
> > I believe it shows clear regression in latest 2.6.25 kernel.
>
> I am still not completely sure. It might be a regression, but it also
> might just trigger the bug in ancient version in libc.so.5 which might
> be fixed in some later version [...]

which too is a regression ...

really, lets add a sysctl for this, and a .config option that either
disables or enables it. Then we will default to disabled. (but users can
enable it - and distros can build their kernels with this .config option
enabled)

Ingo

2008-02-05 13:06:53

by Jakub Jelinek

[permalink] [raw]
Subject: Re: [regression] Re: brk randomization breaks columns

On Tue, Feb 05, 2008 at 01:54:26PM +0100, Ingo Molnar wrote:
> * Jiri Kosina <[email protected]> wrote:
>
> > On Tue, 5 Feb 2008, Pavel Machek wrote:
> >
> > > > Actually, this clearly shows that either prehistoric libc.so.5 or the
> > > > program itself are broken.
> > > I believe it shows clear regression in latest 2.6.25 kernel.
> >
> > I am still not completely sure. It might be a regression, but it also
> > might just trigger the bug in ancient version in libc.so.5 which might
> > be fixed in some later version [...]
>
> which too is a regression ...
>
> really, lets add a sysctl for this, and a .config option that either
> disables or enables it. Then we will default to disabled. (but users can
> enable it - and distros can build their kernels with this .config option
> enabled)

I don't think kernel should care about programs which are buggy and make invalid
assumptions, and that's the case here. I remember we have been through this
5 years ago when brk randomization has been added to Red Hat kernels. There was
one or two broken programs which made assumptions on what brk(0) is supposed
to return at program startup, everything else was ok.
For the buggy apps there is always setarch i386 -R ./the_buggy_program
so I don't think we need to add another sysctl for this.

Jakub

2008-02-05 13:10:17

by Hugh Dickins

[permalink] [raw]
Subject: Re: brk randomization breaks columns

On Tue, 5 Feb 2008, Jiri Kosina wrote:
> Now, you are right that the return value from brk() is bogus in these
> cases. The patch below should make it behave, as you can easily check with
> strace, right? Does anyone have any comments regarding this patch please?

Your patch below looks good to me, whatever the outcome of the
randomize_brk debate. I can't imagine what end_code has to do
with it: an ELF binary that specifies the code is to go up the
top of the address space seems perfectly reasonable to me.

> Still, it will probably not fix your particular program crashes, just
> because it will always assume that brk starts immediately after the end of
> the bss, which is plain wrong and has never been assured. Could you please
> check whether there is any compat-* package available for you
> distribution, that upgrades libc.so.5 to any fixed version?

But I was myself surprised by your randomize_brk patch: like the
buggy program, I'd imagined that data immediately followed by bss
immediately followed by brk was an invariant (whereas I never
supposed the position of the code had anything to do with it).
Just my ignorance, but not surprising if it's shared by others.

So, I didn't really expect the randomize_brk patch to get this far,
thought it would hit trouble earlier. What to do now? On the one
hand Pavel rightly regards this as a regression, on the other hand
we've had programs which make bad assumptions about their address
space layout before, and have not always deferred to them.

If such cases are few (can we be sure of that yet?), is it going
to be good enough to rely on adding the ELF note to disable their
randomization?

In my usual dither, I'm rather hoping Arjan will have a clear answer.

Hugh

> 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.
>
> Signed-off-by: Jiri Kosina <[email protected]>

Acked-by: Hugh Dickins <[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-05 15:00:37

by Arjan van de Ven

[permalink] [raw]
Subject: Re: brk randomization breaks columns

>
> But I was myself surprised by your randomize_brk patch: like the
> buggy program, I'd imagined that data immediately followed by bss
> immediately followed by brk was an invariant (whereas I never
> supposed the position of the code had anything to do with it).
> Just my ignorance, but not surprising if it's shared by others.

the surprise also will be when gcc reorders your .data section for optimization,
and the variable you think is last.. no longer isn't.

>
> So, I didn't really expect the randomize_brk patch to get this far,
> thought it would hit trouble earlier. What to do now? On the one
> hand Pavel rightly regards this as a regression, on the other hand
> we've had programs which make bad assumptions about their address
> space layout before, and have not always deferred to them.
>
> If such cases are few (can we be sure of that yet?), is it going

it's for sure only few; this is the second known case is 5 years
that made this dicey assumption (RHEL/Fedora ship with this for 5 years or so now)
Various "secure" distros ship with brk detached as well (albeit via other methods)
for even longer.

> to be good enough to rely on adding the ELF note to disable their
> randomization?

sadly elfnotes are a sparse commodity.
>
> In my usual dither, I'm rather hoping Arjan will have a clear answer.


setarch works. If the apps come in source form they need fixing anyway (since I'd not be
surprised of current gcc reorders variables), if not.. we only have 2 cases,
the other case was the build process of emacs (which got fixed 5 years ago).
My personal guess is, if there's nothing more than "columns", lets swallow it.
If more show up, we need to rethink it. I somehow doubt more real apps will show
up given how widespread and long this patch was deployed... but turning of randomization
already exists via various methods, both global and per process.

2008-02-05 15:46:58

by Pavel Machek

[permalink] [raw]
Subject: Re: brk randomization breaks columns

Hi!

> > In my usual dither, I'm rather hoping Arjan will have a clear answer.
>
>
> setarch works. If the apps come in source form they need fixing anyway (since I'd not be
> surprised of current gcc reorders variables), if not.. we only have 2 cases,
> the other case was the build process of emacs (which got fixed 5
> years ago).

uemacs ... broken with randomization
colums, sss ... local programs, broken with randomization
procinfo ... broken, randomization makes it die sooner.
mikmod ... broken with randomization
bsdsed ... broken with randomization
...
Should I test few more?
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

2008-02-05 15:49:21

by Jiri Kosina

[permalink] [raw]
Subject: Re: brk randomization breaks columns

On Tue, 5 Feb 2008, Pavel Machek wrote:

> uemacs ... broken with randomization
> colums, sss ... local programs, broken with randomization
> procinfo ... broken, randomization makes it die sooner.
> mikmod ... broken with randomization
> bsdsed ... broken with randomization
> ...
> Should I test few more?

Just to make things clear -- are all of these linked against the 1996
version of libc.so.5?

Thanks,

--
Jiri Kosina
SUSE Labs

2008-02-05 15:50:31

by Ingo Molnar

[permalink] [raw]
Subject: Re: brk randomization breaks columns


* Pavel Machek <[email protected]> wrote:

> Hi!
>
> > > In my usual dither, I'm rather hoping Arjan will have a clear answer.
> >
> >
> > setarch works. If the apps come in source form they need fixing anyway (since I'd not be
> > surprised of current gcc reorders variables), if not.. we only have 2 cases,
> > the other case was the build process of emacs (which got fixed 5
> > years ago).
>
> uemacs ... broken with randomization
> colums, sss ... local programs, broken with randomization
> procinfo ... broken, randomization makes it die sooner.
> mikmod ... broken with randomization
> bsdsed ... broken with randomization
> ...
> Should I test few more?

just a quick debug question: do they all work if you start them via
setarch?

Ingo

2008-02-05 15:55:33

by Pavel Machek

[permalink] [raw]
Subject: Re: brk randomization breaks columns

On Tue 2008-02-05 16:49:04, Jiri Kosina wrote:
> On Tue, 5 Feb 2008, Pavel Machek wrote:
>
> > uemacs ... broken with randomization
> > colums, sss ... local programs, broken with randomization
> > procinfo ... broken, randomization makes it die sooner.
> > mikmod ... broken with randomization
> > bsdsed ... broken with randomization
> > ...
> > Should I test few more?
>
> Just to make things clear -- are all of these linked against the 1996
> version of libc.so.5?

Yes.

root@amd:~# ls -al /lib/libc.so.5.4.33
-rwxr-xr-x 1 root root 679259 Aug 2 1997 /lib/libc.so.5.4.33*

Pavel

(Hmm... I should really cleanup my system:

pavel@amd:/usr/local/bin$ ./archie
-sh: ./archie: cannot execute binary file
pavel@amd:/usr/local/bin$ file ./archie
./archie: Linux/i386 demand-paged executable (QMAGIC))

--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

2008-02-05 16:00:51

by Pavel Machek

[permalink] [raw]
Subject: Re: brk randomization breaks columns

On Tue 2008-02-05 16:49:53, Ingo Molnar wrote:
>
> * Pavel Machek <[email protected]> wrote:
>
> > Hi!
> >
> > > > In my usual dither, I'm rather hoping Arjan will have a clear answer.
> > >
> > >
> > > setarch works. If the apps come in source form they need fixing anyway (since I'd not be
> > > surprised of current gcc reorders variables), if not.. we only have 2 cases,
> > > the other case was the build process of emacs (which got fixed 5
> > > years ago).
> >
> > uemacs ... broken with randomization
> > colums, sss ... local programs, broken with randomization
> > procinfo ... broken, randomization makes it die sooner.
> > mikmod ... broken with randomization
> > bsdsed ... broken with randomization
> > ...
> > Should I test few more?
>
> just a quick debug question: do they all work if you start them via
> setarch?

I was actually toggling randomization with

echo 0|1 > /proc/sys/kernel/randomize_va_space

. Yes, setarch i386 -R /usr/local/bin/uemacs (etc) fixes them, too.

What about this?

Heap randomization breaks /lib/libc.so.5.4.33, make it possible to
randomize normal stuff but leave the heap alone.

Signed-off-by: Pavel Machek <[email protected]>


diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
index 18ed6dd..4b099ea 100644
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
@@ -1077,7 +1077,7 @@ #endif /* ARCH_HAS_SETUP_ADDITIONAL_PAGE
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

--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

2008-02-05 16:03:34

by Pavel Machek

[permalink] [raw]
Subject: Re: brk randomization breaks columns

Hi!

> Still, it will probably not fix your particular program crashes, just
> because it will always assume that brk starts immediately after the end of
> the bss, which is plain wrong and has never been assured. Could you please
> check whether there is any compat-* package available for you
> distribution, that upgrades libc.so.5 to any fixed version?

I got libc.so.5 as a binary from some Slackware-based system, IIRC,
and I can't find any relevant compat-* packages :-(.

> 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.
>
> Signed-off-by: Jiri Kosina <[email protected]>

ACK.


> 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;
>
> /*

--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

2008-02-05 16:06:48

by Ingo Molnar

[permalink] [raw]
Subject: Re: brk randomization breaks columns


* Pavel Machek <[email protected]> wrote:

> . Yes, setarch i386 -R /usr/local/bin/uemacs (etc) fixes them, too.
>
> What about this?
>
> Heap randomization breaks /lib/libc.so.5.4.33, make it possible to
> randomize normal stuff but leave the heap alone.

certainly looks fine to me, but please also add a .config to make it
default to 2. The reason is that a good portions of the overflows happen
on the heap and 99.9% of the Linux users do not run 1996-era glibc
anymore.

something like CONFIG_COMPAT_BRK=y, which would cause randomize_va_space
to default to 1, and if a user or distro disables it, it will default to
2.

Ingo

2008-02-05 16:09:41

by Ingo Molnar

[permalink] [raw]
Subject: Re: brk randomization breaks columns


* Pavel Machek <[email protected]> wrote:

> > 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.
> >
> > Signed-off-by: Jiri Kosina <[email protected]>
>
> ACK.

actually, does the patch above from Jiri solve the problem with your 11
year old binaries too? (Sorry if it has been mentioned elsewhere, i
could not decode it from this thread.)

Ingo

2008-02-05 16:13:00

by Pavel Machek

[permalink] [raw]
Subject: Re: [regression] Re: brk randomization breaks columns

On Tue 2008-02-05 13:50:51, Jiri Kosina wrote:
> On Tue, 5 Feb 2008, Pavel Machek wrote:
>
> > > Actually, this clearly shows that either prehistoric libc.so.5 or the
> > > program itself are broken.
> > I believe it shows clear regression in latest 2.6.25 kernel.
>
> I am still not completely sure. It might be a regression, but it also
> might just trigger the bug in ancient version in libc.so.5 which might be
> fixed in some later version -- are you able to verify that?

I'm in same position as you here. I only have few old binaries :-(.

> > You say it is wrong. Manpages imply otherwise:
> >
> > int brk(void *end_data_segment);
> > ...
> > DESCRIPTION
> > brk() sets the end of the data segment to the value specified by
> > end_data_segment, when that value is reasonable, the system does have enough
> > memory and the process does not exceed its max data size (see setrlimit(2)).
> > Note it talks about data segment, not about heap, and that seems to
> > imply that BSS and heap are actually one area. 2.6.25 broke that.
>
> Single Unix Specification talks only about manipulating the break section,
> see http://opengroup.org/onlinepubs/007908775/xsh/brk.html

SuS:
# The brk() and sbrk() functions are used to change the amount of space
# allocated for the calling process.

It talks about "space for calling process". It does not talk about
heap, and I think it implicitely assumes "bss and heap" are
continuous...

(It also says return 0 or success, and we return address).

> > > Still, it will probably not fix your particular program crashes, just
> > > because it will always assume that brk starts immediately after the end of
> > > the bss, which is plain wrong and has never been assured. Could you please
> > Can you quote docs that tells me it is plain wrong?
>
> See the Single Unix Specification. It doesn't seem to allow you to assume
> *anything* about start_brk location, seems to me.

I believe it implicitely assumes start_brk is well known,
actually. Otherwise it should have told us how to get start_brk.
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

2008-02-05 16:18:27

by Pavel Machek

[permalink] [raw]
Subject: Re: [regression] Re: brk randomization breaks columns

On Tue 2008-02-05 08:05:46, Jakub Jelinek wrote:
> On Tue, Feb 05, 2008 at 01:54:26PM +0100, Ingo Molnar wrote:
> > * Jiri Kosina <[email protected]> wrote:
> >
> > > On Tue, 5 Feb 2008, Pavel Machek wrote:
> > >
> > > > > Actually, this clearly shows that either prehistoric libc.so.5 or the
> > > > > program itself are broken.
> > > > I believe it shows clear regression in latest 2.6.25 kernel.
> > >
> > > I am still not completely sure. It might be a regression, but it also
> > > might just trigger the bug in ancient version in libc.so.5 which might
> > > be fixed in some later version [...]
> >
> > which too is a regression ...
> >
> > really, lets add a sysctl for this, and a .config option that either
> > disables or enables it. Then we will default to disabled. (but users can
> > enable it - and distros can build their kernels with this .config option
> > enabled)
>
> I don't think kernel should care about programs which are buggy and make invalid
> assumptions, and that's the case here. I remember we have been

Those "invalid assumptions" crept into documentation. Everybody knew
heap starts at the end of bss in 1995.

> 5 years ago when brk randomization has been added to Red Hat kernels. There was
> one or two broken programs which made assumptions on what brk(0) is supposed
> to return at program startup, everything else was ok.

That's not the problem. Problem is that programs assume
brk(0x12345678) allocates space between end of bss and 0x12345678;
which is no longer the case.

And actually even
http://opengroup.org/onlinepubs/007908775/xsh/brk.html only talks
about "ammount of space"... implying begging of that space is well
known.

Pavel
PS: It would be nice to fix linux man pages to say that it brk() moves
end of the heap, only, and that any usage of brk() is invalid w/o
doing brk(0) before.

--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

2008-02-05 16:38:19

by Ingo Molnar

[permalink] [raw]
Subject: Re: [regression] Re: brk randomization breaks columns


* Pavel Machek <[email protected]> wrote:

> > > > I am still not completely sure. It might be a regression, but it
> > > > also might just trigger the bug in ancient version in libc.so.5
> > > > which might be fixed in some later version [...]
> > >
> > > which too is a regression ...
> > >
> > > really, lets add a sysctl for this, and a .config option that
> > > either disables or enables it. Then we will default to disabled.
> > > (but users can enable it - and distros can build their kernels
> > > with this .config option enabled)
> >
> > I don't think kernel should care about programs which are buggy and
> > make invalid assumptions, and that's the case here. I remember we
> > have been
>
> Those "invalid assumptions" crept into documentation. Everybody knew
> heap starts at the end of bss in 1995.

what matters most isnt really any documentation but what programs really
do, and how it affects users.

in this case i think we should offer a .config option to set the
randomization behavior, and should perhaps make the more conservative
one the default.

New distros (with 10,000+ binaries that work just fine with
randomization) will turn on max randomization by default, while people
like you who mix 1996 binaries with a 2008 kernel will use a more
conservative default.

Ingo

2008-02-05 16:59:18

by Arjan van de Ven

[permalink] [raw]
Subject: Re: brk randomization breaks columns

On Tue, 5 Feb 2008 16:46:48 +0100
Pavel Machek <[email protected]> wrote:

> Hi!
>
> > > In my usual dither, I'm rather hoping Arjan will have a clear
> > > answer.
> >
> >
> > setarch works. If the apps come in source form they need fixing
> > anyway (since I'd not be surprised of current gcc reorders
> > variables), if not.. we only have 2 cases, the other case was the
> > build process of emacs (which got fixed 5 years ago).
>
> uemacs ... broken with randomization
> colums, sss ... local programs, broken with randomization
> procinfo ... broken, randomization makes it die sooner.
> mikmod ... broken with randomization
> bsdsed ... broken with randomization
> ...
> Should I test few more?

ok throw that idea out of the window then

the combo of a config option + sysctl sounds the right way forward then ;(

Unless there's a way we can make sys_brk() detect this kind of behavior somehow...
we could track per process if brk(0) was called, and if not, do something fancy to work around stuff?



--
If you want to reach me at my work email, use [email protected]
For development, discussion and tips for power savings,
visit http://www.lesswatts.org

2008-02-05 17:34:05

by Pavel Machek

[permalink] [raw]
Subject: Re: brk randomization breaks columns

On Tue 2008-02-05 08:58:41, Arjan van de Ven wrote:
> On Tue, 5 Feb 2008 16:46:48 +0100
> Pavel Machek <[email protected]> wrote:
>
> > Hi!
> >
> > > > In my usual dither, I'm rather hoping Arjan will have a clear
> > > > answer.
> > >
> > >
> > > setarch works. If the apps come in source form they need fixing
> > > anyway (since I'd not be surprised of current gcc reorders
> > > variables), if not.. we only have 2 cases, the other case was the
> > > build process of emacs (which got fixed 5 years ago).
> >
> > uemacs ... broken with randomization
> > colums, sss ... local programs, broken with randomization
> > procinfo ... broken, randomization makes it die sooner.
> > mikmod ... broken with randomization
> > bsdsed ... broken with randomization
> > ...
> > Should I test few more?
>
> ok throw that idea out of the window then
>
> the combo of a config option + sysctl sounds the right way forward then ;(
>
> Unless there's a way we can make sys_brk() detect this kind of behavior somehow...
> we could track per process if brk(0) was called, and if not, do something fancy to work around stuff?

That behaviour is indeed easy to detect. If someone does sys_brk()
with value lower than heap_start, it means it is old binary (but I'd
call that a hack).

But... older binaries call sys_personality(). Maybe it is cleaner not
to base check on that?
Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

2008-02-05 18:05:24

by Pavel Machek

[permalink] [raw]
Subject: Re: brk randomization breaks columns

Hi!

> 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;
>
> /*

Sorry, I now tested the patch. It looked good to my untrained eyes,
but upon testing it on columns:

...
personality(PER_LINUX) = 4194304
geteuid() = 1000
getuid() = 1000
getgid() = 1002
getegid() = 1002
brk(0x8054098) = 0x922d000
--- SIGSEGV (Segmentation fault) @ 0 (0) ---

...it should have said -EINVAL or something like that.

Pavel
--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

2008-02-05 20:42:38

by Jiri Kosina

[permalink] [raw]
Subject: Re: brk randomization breaks columns

On Tue, 5 Feb 2008, Pavel Machek wrote:

> Sorry, I now tested the patch. It looked good to my untrained eyes, but
> upon testing it on columns:
> ...
> personality(PER_LINUX) = 4194304
> geteuid() = 1000
> getuid() = 1000
> getgid() = 1002
> getegid() = 1002
> brk(0x8054098) = 0x922d000
> --- SIGSEGV (Segmentation fault) @ 0 (0) ---
> ...it should have said -EINVAL or something like that.

I don't think so.

brk() should return the value of the current break when passed an
unreasonable argument (as in the case you ilustrated -- it tried to set
the brk lower than start_brk, which doesn't make sense). The only error
values brk() is allowed to return are ENOMEM and EAGAIN, which don't fit
into this situation. This is desgribed both in SUS and linux manpages.

--
Jiri Kosina
SUSE Labs

2008-02-05 22:03:32

by Pavel Machek

[permalink] [raw]
Subject: Re: brk randomization breaks columns

Hi!

> > . Yes, setarch i386 -R /usr/local/bin/uemacs (etc) fixes them, too.
> >
> > What about this?
> >
> > Heap randomization breaks /lib/libc.so.5.4.33, make it possible to
> > randomize normal stuff but leave the heap alone.
>
> certainly looks fine to me, but please also add a .config to make it
> default to 2. The reason is that a good portions of the overflows happen
> on the heap and 99.9% of the Linux users do not run 1996-era glibc
> anymore.
>
> something like CONFIG_COMPAT_BRK=y, which would cause randomize_va_space
> to default to 1, and if a user or distro disables it, it will default to
> 2.

I named it slightly differently, but idea is same. Heap randomization
breaks ancient binaries, so disable it by default.

Signed-off-by: Pavel Machek <[email protected]>

diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c
index 18ed6dd..4b099ea 100644
--- a/fs/binfmt_elf.c
+++ b/fs/binfmt_elf.c
@@ -1077,7 +1077,7 @@ #endif /* ARCH_HAS_SETUP_ADDITIONAL_PAGE
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
diff --git a/init/Kconfig b/init/Kconfig
index dcc96a8..37c7852 100644
--- a/init/Kconfig
+++ b/init/Kconfig
@@ -541,6 +541,16 @@ config ELF_CORE
help
Enable support for generating core dumps. Disabling saves about 4k.

+config RANDOMIZE_HEAP
+ bool "Randomize heap placement"
+ help
+ Randomizing heap placement makes heap exploits harder, but it also
+ breaks old programs (including anything libc5 based). This only
+ changes default, and can be overriden in runtime by tweaking
+ /proc/sys/kernel/randomize_va_space.
+
+ N is safe choice here.
+
config BASE_FULL
default y
bool "Enable full-sized data structures for core" if EMBEDDED
diff --git a/mm/memory.c b/mm/memory.c
index 4706af1..5134014 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -82,7 +82,12 @@ void * high_memory;
EXPORT_SYMBOL(num_physpages);
EXPORT_SYMBOL(high_memory);

-int randomize_va_space __read_mostly = 1;
+int randomize_va_space __read_mostly =
+#ifdef CONFIG_RANDOMIZE_HEAP
+ 2;
+#else
+ 1;
+#endif

static int __init disable_randmaps(char *s)
{


--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

2008-02-05 22:04:21

by Pavel Machek

[permalink] [raw]
Subject: Re: brk randomization breaks columns

On Tue 2008-02-05 17:09:13, Ingo Molnar wrote:
>
> * Pavel Machek <[email protected]> wrote:
>
> > > 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.
> > >
> > > Signed-off-by: Jiri Kosina <[email protected]>
> >
> > ACK.
>
> actually, does the patch above from Jiri solve the problem with your 11
> year old binaries too? (Sorry if it has been mentioned elsewhere, i
> could not decode it from this thread.)

No, it should just make the problem more obvious...

--
(english) http://www.livejournal.com/~pavelmachek
(cesky, pictures) http://atrey.karlin.mff.cuni.cz/~pavel/picture/horses/blog.html

2008-02-05 22:35:38

by Jiri Kosina

[permalink] [raw]
Subject: Re: brk randomization breaks columns

On Tue, 5 Feb 2008, Arjan van de Ven wrote:

> the combo of a config option + sysctl sounds the right way forward then
> ;(

OK, so I propose the one below (unested yet, but should be trivial). Does
anyone have any objections?



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.

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

diff --git a/Documentation/sysctl/kernel.txt b/Documentation/sysctl/kernel.txt
index 8984a53..0373bbe 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 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 chose 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..804a3a6 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 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 chose 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..5765567 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_RANDOMIZE_CONSERVATIVE
int randomize_va_space __read_mostly = 1;
+#else
+#ifdef CONFIG_RANDOMIZE_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 03:26:21

by Randy Dunlap

[permalink] [raw]
Subject: Re: brk randomization breaks columns

On Tue, 5 Feb 2008 23:35:27 +0100 (CET) Jiri Kosina wrote:

> On Tue, 5 Feb 2008, Arjan van de Ven wrote:
>
> > the combo of a config option + sysctl sounds the right way forward then
> > ;(
>
> OK, so I propose the one below (unested yet, but should be trivial). Does
> anyone have any objections?
>
>
>
> 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.

Thanks.

> Signed-off-by: Jiri Kosina <[email protected]>
>
> diff --git a/Documentation/sysctl/kernel.txt b/Documentation/sysctl/kernel.txt
> index 8984a53..0373bbe 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 is used in the system, for architectures

s/is used/that is used/

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

Drop comma ^ that the brk area

> + 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 chose Full randomization.

choose

> +
> +==============================================================
> +
> reboot-cmd: (Sparc only)
>
> ??? This seems to be a way to give an argument to the Sparc

> diff --git a/init/Kconfig b/init/Kconfig
> index 87f50df..804a3a6 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 brk area starts

Drop comma. s/that brk/that the brk/

> + 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 chose Full randomization.

s/chose/choose/

> + Equivalent to sysctl kernel.randomize_va_space = 2.
> +
> +endchoice
> +
> config PROFILING
> bool "Profiling support (EXPERIMENTAL)"
> help

---
~Randy