* [PATCH] brk: fix min_brk lower bound computation for COMPAT_BRK @ 2011-01-02 23:05 Jiri Kosina 2011-01-03 11:39 ` Jiri Kosina 0 siblings, 1 reply; 15+ messages in thread From: Jiri Kosina @ 2011-01-02 23:05 UTC (permalink / raw) To: Ingo Molnar, Andrew Morton; +Cc: Geert Uytterhoeven, linux-kernel Even if CONFIG_COMPAT_BRK is set in the kernel configuration, it can still be overriden by randomize_va_space sysctl. If this is the case, the min_brk computation in sys_brk() implementation is wrong, as it solely takes into account COMPAT_BRK setting, assuming that brk start is not randomized. But that might not be the case if randomize_va_space sysctl has been set to '2' at the time the binary has been loaded from disk. In such case, the check has to be done in a same way as in !CONFIG_COMPAT_BRK case. Signed-off-by: Jiri Kosina <jkosina@suse.cz> --- mm/mmap.c | 10 +++++++++- 1 files changed, 9 insertions(+), 1 deletions(-) diff --git a/mm/mmap.c b/mm/mmap.c index 50a4aa0..35d9f9c 100644 --- a/mm/mmap.c +++ b/mm/mmap.c @@ -253,7 +253,15 @@ SYSCALL_DEFINE1(brk, unsigned long, brk) down_write(&mm->mmap_sem); #ifdef CONFIG_COMPAT_BRK - min_brk = mm->end_code; + /* + * CONFIG_COMPAT_BRK can still be overridden by setting + * randomize_va_space to 2, which will still make mm->start_brk + * to be arbitrarily shifted + */ + if (mm->start_brk > mm->end_code) + min_brk = mm->start_brk; + else + min_brk = mm->end_code; #else min_brk = mm->start_brk; #endif -- 1.7.3.1 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH] brk: fix min_brk lower bound computation for COMPAT_BRK 2011-01-02 23:05 [PATCH] brk: fix min_brk lower bound computation for COMPAT_BRK Jiri Kosina @ 2011-01-03 11:39 ` Jiri Kosina 2011-01-03 13:24 ` [PATCH v2] " Jiri Kosina 0 siblings, 1 reply; 15+ messages in thread From: Jiri Kosina @ 2011-01-03 11:39 UTC (permalink / raw) To: Ingo Molnar, Andrew Morton; +Cc: Geert Uytterhoeven, linux-kernel On Mon, 3 Jan 2011, Jiri Kosina wrote: > Even if CONFIG_COMPAT_BRK is set in the kernel configuration, it can still > be overriden by randomize_va_space sysctl. > > If this is the case, the min_brk computation in sys_brk() implementation > is wrong, as it solely takes into account COMPAT_BRK setting, assuming > that brk start is not randomized. But that might not be the case if > randomize_va_space sysctl has been set to '2' at the time the binary has > been loaded from disk. > > In such case, the check has to be done in a same way as in > !CONFIG_COMPAT_BRK case. > > Signed-off-by: Jiri Kosina <jkosina@suse.cz> > --- > mm/mmap.c | 10 +++++++++- > 1 files changed, 9 insertions(+), 1 deletions(-) > > diff --git a/mm/mmap.c b/mm/mmap.c > index 50a4aa0..35d9f9c 100644 > --- a/mm/mmap.c > +++ b/mm/mmap.c > @@ -253,7 +253,15 @@ SYSCALL_DEFINE1(brk, unsigned long, brk) > down_write(&mm->mmap_sem); > > #ifdef CONFIG_COMPAT_BRK > - min_brk = mm->end_code; > + /* > + * CONFIG_COMPAT_BRK can still be overridden by setting > + * randomize_va_space to 2, which will still make mm->start_brk > + * to be arbitrarily shifted > + */ > + if (mm->start_brk > mm->end_code) > + min_brk = mm->start_brk; > + else > + min_brk = mm->end_code; > #else > min_brk = mm->start_brk; > #endif Hmm, actually no, that will bring us back into pre-a5b4592c era, please disregard that. I will have to come up with something more clever (we can't just check 'randomize_va_space == 2', as that would be racy for already running programs). -- Jiri Kosina SUSE Labs, Novell Inc. ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH v2] brk: fix min_brk lower bound computation for COMPAT_BRK 2011-01-03 11:39 ` Jiri Kosina @ 2011-01-03 13:24 ` Jiri Kosina 2011-03-24 21:49 ` [regression v2.6.38] " Geert Uytterhoeven 0 siblings, 1 reply; 15+ messages in thread From: Jiri Kosina @ 2011-01-03 13:24 UTC (permalink / raw) To: Ingo Molnar, Andrew Morton; +Cc: Geert Uytterhoeven, linux-kernel > > diff --git a/mm/mmap.c b/mm/mmap.c > > index 50a4aa0..35d9f9c 100644 > > --- a/mm/mmap.c > > +++ b/mm/mmap.c > > @@ -253,7 +253,15 @@ SYSCALL_DEFINE1(brk, unsigned long, brk) > > down_write(&mm->mmap_sem); > > > > #ifdef CONFIG_COMPAT_BRK > > - min_brk = mm->end_code; > > + /* > > + * CONFIG_COMPAT_BRK can still be overridden by setting > > + * randomize_va_space to 2, which will still make mm->start_brk > > + * to be arbitrarily shifted > > + */ > > + if (mm->start_brk > mm->end_code) > > + min_brk = mm->start_brk; > > + else > > + min_brk = mm->end_code; > > #else > > min_brk = mm->start_brk; > > #endif > > Hmm, actually no, that will bring us back into pre-a5b4592c era, please > disregard that. > > I will have to come up with something more clever (we can't just check > 'randomize_va_space == 2', as that would be racy for already running > programs). The patch below handles all the cases properly. Ingo, Andrew, please consider applying. Thanks. From: Jiri Kosina <jkosina@suse.cz> Subject: [PATCH] brk: fix min_brk lower bound computation for COMPAT_BRK Even if CONFIG_COMPAT_BRK is set in the kernel configuration, it can still be overriden by randomize_va_space sysctl. If this is the case, the min_brk computation in sys_brk() implementation is wrong, as it solely takes into account COMPAT_BRK setting, assuming that brk start is not randomized. But that might not be the case if randomize_va_space sysctl has been set to '2' at the time the binary has been loaded from disk. In such case, the check has to be done in a same way as in !CONFIG_COMPAT_BRK case. In addition to that, the check for the COMPAT_BRK case introduced back in a5b4592c ("brk: make sys_brk() honor COMPAT_BRK when computing lower bound") is slightly wrong -- the lower bound shouldn't be mm->end_code, but mm->end_data instead, as that's where the legacy applications expect brk section to start (i.e. immediately after last global variable). Signed-off-by: Jiri Kosina <jkosina@suse.cz> --- mm/mmap.c | 10 +++++++++- 1 files changed, 9 insertions(+), 1 deletions(-) diff --git a/mm/mmap.c b/mm/mmap.c index 50a4aa0..ca2f164 100644 --- a/mm/mmap.c +++ b/mm/mmap.c @@ -253,7 +253,15 @@ SYSCALL_DEFINE1(brk, unsigned long, brk) down_write(&mm->mmap_sem); #ifdef CONFIG_COMPAT_BRK - min_brk = mm->end_code; + /* + * CONFIG_COMPAT_BRK can still be overridden by setting + * randomize_va_space to 2, which will still make mm->start_brk + * to be arbitrarily shifted + */ + if (mm->start_brk > PAGE_ALIGN(mm->end_data)) + min_brk = mm->start_brk; + else + min_brk = mm->end_data; #else min_brk = mm->start_brk; #endif -- 1.7.3.1 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* [regression v2.6.38] Re: [PATCH v2] brk: fix min_brk lower bound computation for COMPAT_BRK 2011-01-03 13:24 ` [PATCH v2] " Jiri Kosina @ 2011-03-24 21:49 ` Geert Uytterhoeven 2011-03-25 10:20 ` Jiri Kosina 0 siblings, 1 reply; 15+ messages in thread From: Geert Uytterhoeven @ 2011-03-24 21:49 UTC (permalink / raw) To: Jiri Kosina; +Cc: Ingo Molnar, Andrew Morton, linux-kernel, Linux/m68k On Mon, Jan 3, 2011 at 14:24, Jiri Kosina <jkosina@suse.cz> wrote: >> > diff --git a/mm/mmap.c b/mm/mmap.c >> > index 50a4aa0..35d9f9c 100644 >> > --- a/mm/mmap.c >> > +++ b/mm/mmap.c >> > @@ -253,7 +253,15 @@ SYSCALL_DEFINE1(brk, unsigned long, brk) >> > down_write(&mm->mmap_sem); >> > >> > #ifdef CONFIG_COMPAT_BRK >> > - min_brk = mm->end_code; >> > + /* >> > + * CONFIG_COMPAT_BRK can still be overridden by setting >> > + * randomize_va_space to 2, which will still make mm->start_brk >> > + * to be arbitrarily shifted >> > + */ >> > + if (mm->start_brk > mm->end_code) >> > + min_brk = mm->start_brk; >> > + else >> > + min_brk = mm->end_code; >> > #else >> > min_brk = mm->start_brk; >> > #endif >> >> Hmm, actually no, that will bring us back into pre-a5b4592c era, please >> disregard that. >> >> I will have to come up with something more clever (we can't just check >> 'randomize_va_space == 2', as that would be racy for already running >> programs). > > The patch below handles all the cases properly. Ingo, Andrew, please > consider applying. Thanks. > > > > From: Jiri Kosina <jkosina@suse.cz> > Subject: [PATCH] brk: fix min_brk lower bound computation for COMPAT_BRK > > Even if CONFIG_COMPAT_BRK is set in the kernel configuration, it can still > be overriden by randomize_va_space sysctl. > > If this is the case, the min_brk computation in sys_brk() implementation > is wrong, as it solely takes into account COMPAT_BRK setting, assuming > that brk start is not randomized. But that might not be the case if > randomize_va_space sysctl has been set to '2' at the time the binary has > been loaded from disk. > > In such case, the check has to be done in a same way as in > !CONFIG_COMPAT_BRK case. > > In addition to that, the check for the COMPAT_BRK case introduced back in > a5b4592c ("brk: make sys_brk() honor COMPAT_BRK when computing lower > bound") is slightly wrong -- the lower bound shouldn't be mm->end_code, > but mm->end_data instead, as that's where the legacy applications expect > brk section to start (i.e. immediately after last global variable). > > Signed-off-by: Jiri Kosina <jkosina@suse.cz> > --- > mm/mmap.c | 10 +++++++++- > 1 files changed, 9 insertions(+), 1 deletions(-) > > diff --git a/mm/mmap.c b/mm/mmap.c > index 50a4aa0..ca2f164 100644 > --- a/mm/mmap.c > +++ b/mm/mmap.c > @@ -253,7 +253,15 @@ SYSCALL_DEFINE1(brk, unsigned long, brk) > down_write(&mm->mmap_sem); > > #ifdef CONFIG_COMPAT_BRK > - min_brk = mm->end_code; > + /* > + * CONFIG_COMPAT_BRK can still be overridden by setting > + * randomize_va_space to 2, which will still make mm->start_brk > + * to be arbitrarily shifted > + */ > + if (mm->start_brk > PAGE_ALIGN(mm->end_data)) > + min_brk = mm->start_brk; > + else > + min_brk = mm->end_data; > #else > min_brk = mm->start_brk; > #endif > -- > 1.7.3.1 Sorry for chiming in this late, but I've just bisected a problem in 2.6.38 to commit 5520e89485252c759ee60d313e9422447659947b ("brk: fix min_brk lower bound computation for COMPAT_BRK"). When booting my very old test ramdisk on Amiga/m68k, it fails like this: | RAMDISK: gzip image found at block 0 | VFS: Mounted root (ext2 filesystem) readonly on device 1:0. | warning: process `update' used the obsolete bdflush system call | Fix your initscripts? | init: cannot open inittab | Kernel panic - not syncing: Attempted to kill init! Sorry for not noticing earlier, I usually boot full Debians under ARAnyM, instead of booting old ramdisks with libc5-based binaries that once were considered new. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [regression v2.6.38] Re: [PATCH v2] brk: fix min_brk lower bound computation for COMPAT_BRK 2011-03-24 21:49 ` [regression v2.6.38] " Geert Uytterhoeven @ 2011-03-25 10:20 ` Jiri Kosina 2011-03-26 13:51 ` Geert Uytterhoeven 0 siblings, 1 reply; 15+ messages in thread From: Jiri Kosina @ 2011-03-25 10:20 UTC (permalink / raw) To: Geert Uytterhoeven; +Cc: Ingo Molnar, Andrew Morton, linux-kernel, Linux/m68k On Thu, 24 Mar 2011, Geert Uytterhoeven wrote: > > From: Jiri Kosina <jkosina@suse.cz> > > Subject: [PATCH] brk: fix min_brk lower bound computation for COMPAT_BRK > > > > Even if CONFIG_COMPAT_BRK is set in the kernel configuration, it can still > > be overriden by randomize_va_space sysctl. > > > > If this is the case, the min_brk computation in sys_brk() implementation > > is wrong, as it solely takes into account COMPAT_BRK setting, assuming > > that brk start is not randomized. But that might not be the case if > > randomize_va_space sysctl has been set to '2' at the time the binary has > > been loaded from disk. > > > > In such case, the check has to be done in a same way as in > > !CONFIG_COMPAT_BRK case. > > > > In addition to that, the check for the COMPAT_BRK case introduced back in > > a5b4592c ("brk: make sys_brk() honor COMPAT_BRK when computing lower > > bound") is slightly wrong -- the lower bound shouldn't be mm->end_code, > > but mm->end_data instead, as that's where the legacy applications expect > > brk section to start (i.e. immediately after last global variable). > > > > Signed-off-by: Jiri Kosina <jkosina@suse.cz> > > --- > > mm/mmap.c | 10 +++++++++- > > 1 files changed, 9 insertions(+), 1 deletions(-) > > > > diff --git a/mm/mmap.c b/mm/mmap.c > > index 50a4aa0..ca2f164 100644 > > --- a/mm/mmap.c > > +++ b/mm/mmap.c > > @@ -253,7 +253,15 @@ SYSCALL_DEFINE1(brk, unsigned long, brk) > > down_write(&mm->mmap_sem); > > > > #ifdef CONFIG_COMPAT_BRK > > - min_brk = mm->end_code; > > + /* > > + * CONFIG_COMPAT_BRK can still be overridden by setting > > + * randomize_va_space to 2, which will still make mm->start_brk > > + * to be arbitrarily shifted > > + */ > > + if (mm->start_brk > PAGE_ALIGN(mm->end_data)) > > + min_brk = mm->start_brk; > > + else > > + min_brk = mm->end_data; > > #else > > min_brk = mm->start_brk; > > #endif > > -- > > 1.7.3.1 > > Sorry for chiming in this late, but I've just bisected a problem in > 2.6.38 to commit > 5520e89485252c759ee60d313e9422447659947b ("brk: fix min_brk lower bound > computation for COMPAT_BRK"). > > When booting my very old test ramdisk on Amiga/m68k, it fails like this: > > | RAMDISK: gzip image found at block 0 > | VFS: Mounted root (ext2 filesystem) readonly on device 1:0. > | warning: process `update' used the obsolete bdflush system call > | Fix your initscripts? > | init: cannot open inittab > | Kernel panic - not syncing: Attempted to kill init! > > Sorry for not noticing earlier, I usually boot full Debians under ARAnyM, > instead of booting old ramdisks with libc5-based binaries that once were > considered new. Oh well, one has to love the libc5-based binaries indeed. Is the patch below fixing the issue you are seeing on your Amiga/m68k? Thanks. diff --git a/mm/mmap.c b/mm/mmap.c index 2ec8eb5..0a02531 100644 --- a/mm/mmap.c +++ b/mm/mmap.c @@ -262,7 +262,7 @@ SYSCALL_DEFINE1(brk, unsigned long, brk) if (mm->start_brk > PAGE_ALIGN(mm->end_data)) min_brk = mm->start_brk; else - min_brk = mm->end_data; + min_brk = mm->end_code; #else min_brk = mm->start_brk; #endif -- Jiri Kosina SUSE Labs, Novell Inc. ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [regression v2.6.38] Re: [PATCH v2] brk: fix min_brk lower bound computation for COMPAT_BRK 2011-03-25 10:20 ` Jiri Kosina @ 2011-03-26 13:51 ` Geert Uytterhoeven 2011-03-28 14:20 ` Jiri Kosina 2011-03-29 12:02 ` [regression v2.6.38] Re: [PATCH v2] brk: fix min_brk lower bound computation for COMPAT_BRK Jiri Kosina 0 siblings, 2 replies; 15+ messages in thread From: Geert Uytterhoeven @ 2011-03-26 13:51 UTC (permalink / raw) To: Jiri Kosina; +Cc: Ingo Molnar, Andrew Morton, linux-kernel, Linux/m68k Hi Jiri, On Fri, Mar 25, 2011 at 11:20, Jiri Kosina <jkosina@suse.cz> wrote: > On Thu, 24 Mar 2011, Geert Uytterhoeven wrote: >> > From: Jiri Kosina <jkosina@suse.cz> >> > Subject: [PATCH] brk: fix min_brk lower bound computation for COMPAT_BRK >> > >> > Even if CONFIG_COMPAT_BRK is set in the kernel configuration, it can still >> > be overriden by randomize_va_space sysctl. >> > >> > If this is the case, the min_brk computation in sys_brk() implementation >> > is wrong, as it solely takes into account COMPAT_BRK setting, assuming >> > that brk start is not randomized. But that might not be the case if >> > randomize_va_space sysctl has been set to '2' at the time the binary has >> > been loaded from disk. >> > >> > In such case, the check has to be done in a same way as in >> > !CONFIG_COMPAT_BRK case. >> > >> > In addition to that, the check for the COMPAT_BRK case introduced back in >> > a5b4592c ("brk: make sys_brk() honor COMPAT_BRK when computing lower >> > bound") is slightly wrong -- the lower bound shouldn't be mm->end_code, >> > but mm->end_data instead, as that's where the legacy applications expect >> > brk section to start (i.e. immediately after last global variable). >> > >> > Signed-off-by: Jiri Kosina <jkosina@suse.cz> >> > --- >> > mm/mmap.c | 10 +++++++++- >> > 1 files changed, 9 insertions(+), 1 deletions(-) >> > >> > diff --git a/mm/mmap.c b/mm/mmap.c >> > index 50a4aa0..ca2f164 100644 >> > --- a/mm/mmap.c >> > +++ b/mm/mmap.c >> > @@ -253,7 +253,15 @@ SYSCALL_DEFINE1(brk, unsigned long, brk) >> > down_write(&mm->mmap_sem); >> > >> > #ifdef CONFIG_COMPAT_BRK >> > - min_brk = mm->end_code; >> > + /* >> > + * CONFIG_COMPAT_BRK can still be overridden by setting >> > + * randomize_va_space to 2, which will still make mm->start_brk >> > + * to be arbitrarily shifted >> > + */ >> > + if (mm->start_brk > PAGE_ALIGN(mm->end_data)) >> > + min_brk = mm->start_brk; >> > + else >> > + min_brk = mm->end_data; >> > #else >> > min_brk = mm->start_brk; >> > #endif >> > -- >> > 1.7.3.1 >> >> Sorry for chiming in this late, but I've just bisected a problem in >> 2.6.38 to commit >> 5520e89485252c759ee60d313e9422447659947b ("brk: fix min_brk lower bound >> computation for COMPAT_BRK"). >> >> When booting my very old test ramdisk on Amiga/m68k, it fails like this: >> >> | RAMDISK: gzip image found at block 0 >> | VFS: Mounted root (ext2 filesystem) readonly on device 1:0. >> | warning: process `update' used the obsolete bdflush system call >> | Fix your initscripts? >> | init: cannot open inittab >> | Kernel panic - not syncing: Attempted to kill init! >> >> Sorry for not noticing earlier, I usually boot full Debians under ARAnyM, >> instead of booting old ramdisks with libc5-based binaries that once were >> considered new. > > Oh well, one has to love the libc5-based binaries indeed. Yeah, binaries from 1996 ;-) > Is the patch below fixing the issue you are seeing on your Amiga/m68k? > Thanks. > > > diff --git a/mm/mmap.c b/mm/mmap.c > index 2ec8eb5..0a02531 100644 > --- a/mm/mmap.c > +++ b/mm/mmap.c > @@ -262,7 +262,7 @@ SYSCALL_DEFINE1(brk, unsigned long, brk) > if (mm->start_brk > PAGE_ALIGN(mm->end_data)) > min_brk = mm->start_brk; > else > - min_brk = mm->end_data; > + min_brk = mm->end_code; > #else > min_brk = mm->start_brk; > #endif Unfortunately not... I added some printk()s: mm->start_brk = 0x8000a000, PAGE_ALIGN(mm->end_data = 0x8000a000) mm->start_brk = 0x8000a000, PAGE_ALIGN(mm->end_data = 0x8000a000) mm->start_brk = 0x8000a000, PAGE_ALIGN(mm->end_data = 0x8000a000) mm->start_brk = 0x8000a000, PAGE_ALIGN(mm->end_data = 0x8000a000) mm->start_brk = 0x80006000, PAGE_ALIGN(mm->end_data = 0x80004000) I.e. just before the failure, "mm->start_brk > PAGE_ALIGN(mm->end_data)" became true. Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [regression v2.6.38] Re: [PATCH v2] brk: fix min_brk lower bound computation for COMPAT_BRK 2011-03-26 13:51 ` Geert Uytterhoeven @ 2011-03-28 14:20 ` Jiri Kosina 2011-03-29 20:24 ` Geert Uytterhoeven 2011-03-29 12:02 ` [regression v2.6.38] Re: [PATCH v2] brk: fix min_brk lower bound computation for COMPAT_BRK Jiri Kosina 1 sibling, 1 reply; 15+ messages in thread From: Jiri Kosina @ 2011-03-28 14:20 UTC (permalink / raw) To: Geert Uytterhoeven; +Cc: Ingo Molnar, Andrew Morton, linux-kernel, Linux/m68k On Sat, 26 Mar 2011, Geert Uytterhoeven wrote: > >> | RAMDISK: gzip image found at block 0 > >> | VFS: Mounted root (ext2 filesystem) readonly on device 1:0. > >> | warning: process `update' used the obsolete bdflush system call > >> | Fix your initscripts? > >> | init: cannot open inittab > >> | Kernel panic - not syncing: Attempted to kill init! > >> > >> Sorry for not noticing earlier, I usually boot full Debians under ARAnyM, > >> instead of booting old ramdisks with libc5-based binaries that once were > >> considered new. > > > > Oh well, one has to love the libc5-based binaries indeed. > > Yeah, binaries from 1996 ;-) > > > Is the patch below fixing the issue you are seeing on your Amiga/m68k? > > Thanks. > > > > > > diff --git a/mm/mmap.c b/mm/mmap.c > > index 2ec8eb5..0a02531 100644 > > --- a/mm/mmap.c > > +++ b/mm/mmap.c > > @@ -262,7 +262,7 @@ SYSCALL_DEFINE1(brk, unsigned long, brk) > > if (mm->start_brk > PAGE_ALIGN(mm->end_data)) > > min_brk = mm->start_brk; > > else > > - min_brk = mm->end_data; > > + min_brk = mm->end_code; > > #else > > min_brk = mm->start_brk; > > #endif > > Unfortunately not... > > I added some printk()s: > > mm->start_brk = 0x8000a000, PAGE_ALIGN(mm->end_data = 0x8000a000) > mm->start_brk = 0x8000a000, PAGE_ALIGN(mm->end_data = 0x8000a000) > mm->start_brk = 0x8000a000, PAGE_ALIGN(mm->end_data = 0x8000a000) > mm->start_brk = 0x8000a000, PAGE_ALIGN(mm->end_data = 0x8000a000) > mm->start_brk = 0x80006000, PAGE_ALIGN(mm->end_data = 0x80004000) > > I.e. just before the failure, "mm->start_brk > PAGE_ALIGN(mm->end_data)" > became true. Hmm, I think we'd need some refresh on what the ancient libc5-based binaries are actually doing. Could you please send me strace of this application (restricting it to brk()/mmap() calls should be enough). Thanks, -- Jiri Kosina SUSE Labs, Novell Inc. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [regression v2.6.38] Re: [PATCH v2] brk: fix min_brk lower bound computation for COMPAT_BRK 2011-03-28 14:20 ` Jiri Kosina @ 2011-03-29 20:24 ` Geert Uytterhoeven 2011-03-29 20:37 ` Andreas Schwab 2011-04-06 20:08 ` Jiri Kosina 0 siblings, 2 replies; 15+ messages in thread From: Geert Uytterhoeven @ 2011-03-29 20:24 UTC (permalink / raw) To: Jiri Kosina; +Cc: Ingo Molnar, Andrew Morton, linux-kernel, Linux/m68k Hi Jiri, On Mon, Mar 28, 2011 at 16:20, Jiri Kosina <jkosina@suse.cz> wrote: > On Sat, 26 Mar 2011, Geert Uytterhoeven wrote: >> >> | RAMDISK: gzip image found at block 0 >> >> | VFS: Mounted root (ext2 filesystem) readonly on device 1:0. >> >> | warning: process `update' used the obsolete bdflush system call >> >> | Fix your initscripts? >> >> | init: cannot open inittab >> >> | Kernel panic - not syncing: Attempted to kill init! > Hmm, I think we'd need some refresh on what the ancient libc5-based > binaries are actually doing. > > Could you please send me strace of this application (restricting it to > brk()/mmap() calls should be enough). I managed to reproduce it inside ARAnyM, by chrooting into the ramdisk image. After adding strace, "strace /sbin/init" for the success case gives: old_mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0xc0004000 old_mmap(NULL, 578, PROT_READ, MAP_SHARED, 3, 0) = 0xc0007000 old_mmap(NULL, 663552, PROT_NONE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0xc0008000 old_mmap(0xc0008000, 426908, PROT_READ|PROT_EXEC, MAP_PRIVATE|MAP_FIXED, 3, 0) = 0xc0008000 old_mmap(0xc0072000, 24256, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED, 3, 0x68000) = 0xc0072000 old_mmap(0xc0078000, 204696, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_ANONYMOUS, -1, 0) = 0xc0078000 brk(0x80005c8e) = 0x80005c8e brk(0x80006000) = 0x80006000 old_mmap(NULL, 1024, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0xc0007000 For the failure case: old_mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0xc0004000 old_mmap(NULL, 578, PROT_READ, MAP_SHARED, 3, 0) = 0xc0007000 old_mmap(NULL, 663552, PROT_NONE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0xc0008000 old_mmap(0xc0008000, 426908, PROT_READ|PROT_EXEC, MAP_PRIVATE|MAP_FIXED, 3, 0) = 0xc0008000 old_mmap(0xc0072000, 24256, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED, 3, 0x68000) = 0xc0072000 old_mmap(0xc0078000, 204696, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_FIXED|MAP_ANONYMOUS, -1, 0) = 0xc0078000 brk(0x80005c8e) = 0x80006000 Major difference: -brk(0x80005c8e) = 0x80005c8e -brk(0x80006000) = 0x80006000 -open("/etc/inittab", O_RDONLY) = 3 -fstat(3, {st_mode=S_IFREG|0644, st_size=140, ...}) = 0 -old_mmap(NULL, 1024, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0xc0007000 -read(3, "tty1:console:/sbin/getty 9600 tt"..., 1024) = 140 -read(3, "", 1024) = 0 +brk(0x80005c8e) = 0x80006000 +open("/dev/console", O_WRONLY) = 3 +write(3, "init: ", 6) = 6 +write(3, "cannot open inittab\n", 20) = 20 close(3) = 0 Seems like the binary doesn't like brk() rounding up the requested value to the next page... Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [regression v2.6.38] Re: [PATCH v2] brk: fix min_brk lower bound computation for COMPAT_BRK 2011-03-29 20:24 ` Geert Uytterhoeven @ 2011-03-29 20:37 ` Andreas Schwab 2011-04-06 20:08 ` Jiri Kosina 1 sibling, 0 replies; 15+ messages in thread From: Andreas Schwab @ 2011-03-29 20:37 UTC (permalink / raw) To: Geert Uytterhoeven Cc: Jiri Kosina, Ingo Molnar, Andrew Morton, linux-kernel, Linux/m68k Geert Uytterhoeven <geert@linux-m68k.org> writes: > Seems like the binary doesn't like brk() rounding up the requested > value to the next page... >From libc-5.4.46/libc/sysdeps/linux/m68k/__sbrk.c: void * __sbrk(ptrdiff_t increment) { if (__init_brk () == 0) { register void * tmp asm ("%d1") = ___brk_addr+increment; __asm__ volatile ("movel %1,%/d0\n\t" "trap #0\n\t" "movel %/d0,%0" :"=g" (___brk_addr) :"i" (SYS_brk),"g" (tmp) : "%d0"); if (___brk_addr == tmp) return tmp-increment; errno = ENOMEM; } return ((void *) -1); } Andreas. -- Andreas Schwab, schwab@linux-m68k.org GPG Key fingerprint = 58CA 54C7 6D53 942B 1756 01D3 44D5 214B 8276 4ED5 "And now for something completely different." ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [regression v2.6.38] Re: [PATCH v2] brk: fix min_brk lower bound computation for COMPAT_BRK 2011-03-29 20:24 ` Geert Uytterhoeven 2011-03-29 20:37 ` Andreas Schwab @ 2011-04-06 20:08 ` Jiri Kosina 2011-04-06 20:23 ` Geert Uytterhoeven 1 sibling, 1 reply; 15+ messages in thread From: Jiri Kosina @ 2011-04-06 20:08 UTC (permalink / raw) To: Geert Uytterhoeven; +Cc: Ingo Molnar, Andrew Morton, linux-kernel, Linux/m68k On Tue, 29 Mar 2011, Geert Uytterhoeven wrote: > I managed to reproduce it inside ARAnyM, by chrooting into the ramdisk image. > After adding strace, "strace /sbin/init" for the success case gives: > > old_mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, > -1, 0) = 0xc0004000 > old_mmap(NULL, 578, PROT_READ, MAP_SHARED, 3, 0) = 0xc0007000 > old_mmap(NULL, 663552, PROT_NONE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0xc0008000 > old_mmap(0xc0008000, 426908, PROT_READ|PROT_EXEC, > MAP_PRIVATE|MAP_FIXED, 3, 0) = 0xc0008000 > old_mmap(0xc0072000, 24256, PROT_READ|PROT_WRITE, > MAP_PRIVATE|MAP_FIXED, 3, 0x68000) = 0xc0072000 > old_mmap(0xc0078000, 204696, PROT_READ|PROT_WRITE, > MAP_PRIVATE|MAP_FIXED|MAP_ANONYMOUS, -1, 0) = 0xc0078000 > brk(0x80005c8e) = 0x80005c8e > brk(0x80006000) = 0x80006000 > old_mmap(NULL, 1024, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, > -1, 0) = 0xc0007000 > > For the failure case: > > old_mmap(NULL, 4096, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, > -1, 0) = 0xc0004000 > old_mmap(NULL, 578, PROT_READ, MAP_SHARED, 3, 0) = 0xc0007000 > old_mmap(NULL, 663552, PROT_NONE, MAP_PRIVATE|MAP_ANONYMOUS, -1, 0) = 0xc0008000 > old_mmap(0xc0008000, 426908, PROT_READ|PROT_EXEC, > MAP_PRIVATE|MAP_FIXED, 3, 0) = 0xc0008000 > old_mmap(0xc0072000, 24256, PROT_READ|PROT_WRITE, > MAP_PRIVATE|MAP_FIXED, 3, 0x68000) = 0xc0072000 > old_mmap(0xc0078000, 204696, PROT_READ|PROT_WRITE, > MAP_PRIVATE|MAP_FIXED|MAP_ANONYMOUS, -1, 0) = 0xc0078000 > brk(0x80005c8e) = 0x80006000 > > Major difference: > > -brk(0x80005c8e) = 0x80005c8e > -brk(0x80006000) = 0x80006000 > -open("/etc/inittab", O_RDONLY) = 3 > -fstat(3, {st_mode=S_IFREG|0644, st_size=140, ...}) = 0 > -old_mmap(NULL, 1024, PROT_READ|PROT_WRITE, MAP_PRIVATE|MAP_ANONYMOUS, > -1, 0) = 0xc0007000 > -read(3, "tty1:console:/sbin/getty 9600 tt"..., 1024) = 140 > -read(3, "", 1024) = 0 > +brk(0x80005c8e) = 0x80006000 > +open("/dev/console", O_WRONLY) = 3 > +write(3, "init: ", 6) = 6 > +write(3, "cannot open inittab\n", 20) = 20 > close(3) = 0 > > Seems like the binary doesn't like brk() rounding up the requested > value to the next page... Could you please test with the patch below? Thanks. From: Jiri Kosina <jkosina@suse.cz> Subject: [PATCH] brk: COMPAT_BRK needs more special handling of legacy applications 5520e89 ("brk: fix min_brk lower bound computation for COMPAT_BRK") tried to get the whole logic of brk randomization for legacy (libc5-based) applications finally right. It turns out that the way to detect whether brk has actually been randomized or not introduced by that patch still doesn't work for those binaries, as reported by Geert. I don't like it, but currently see no better option than a bit flag in task_struct to catch the CONFIG_COMPAT_BRK && randomize_va_space == 2 case. Reported-by: Geert Uytterhoeven <geert@linux-m68k.org> NOT-YET-Signed-off-by: Jiri Kosina <jkosina@suse.cz> --- fs/binfmt_elf.c | 6 +++++- include/linux/sched.h | 3 +++ mm/mmap.c | 2 +- 3 files changed, 9 insertions(+), 2 deletions(-) diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c index f34078d..303983f 100644 --- a/fs/binfmt_elf.c +++ b/fs/binfmt_elf.c @@ -941,9 +941,13 @@ 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) && (randomize_va_space > 1)) + if ((current->flags & PF_RANDOMIZE) && (randomize_va_space > 1)) { current->mm->brk = current->mm->start_brk = arch_randomize_brk(current->mm); +#ifdef CONFIG_COMPAT_BRK + current->brk_randomized = 1; +#endif + } #endif if (current->personality & MMAP_PAGE_ZERO) { diff --git a/include/linux/sched.h b/include/linux/sched.h index 83bd2e2..239d2fe 100644 --- a/include/linux/sched.h +++ b/include/linux/sched.h @@ -1254,6 +1254,9 @@ struct task_struct { #endif struct mm_struct *mm, *active_mm; +#ifdef CONFIG_COMPAT_BRK + unsigned brk_randomized:1; +#endif #if defined(SPLIT_RSS_COUNTING) struct task_rss_stat rss_stat; #endif diff --git a/mm/mmap.c b/mm/mmap.c index 2ec8eb5..318ed2d 100644 --- a/mm/mmap.c +++ b/mm/mmap.c @@ -259,7 +259,7 @@ SYSCALL_DEFINE1(brk, unsigned long, brk) * randomize_va_space to 2, which will still cause mm->start_brk * to be arbitrarily shifted */ - if (mm->start_brk > PAGE_ALIGN(mm->end_data)) + if (current->brk_randomized) min_brk = mm->start_brk; else min_brk = mm->end_data; -- 1.7.3.1 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [regression v2.6.38] Re: [PATCH v2] brk: fix min_brk lower bound computation for COMPAT_BRK 2011-04-06 20:08 ` Jiri Kosina @ 2011-04-06 20:23 ` Geert Uytterhoeven 2011-04-06 20:38 ` [PATCH] brk: COMPAT_BRK: fix detection of randomized brk (was Re: [regression v2.6.38] Re: [PATCH v2] brk: fix min_brk lower bound computation forCOMPAT_BRK) Jiri Kosina 0 siblings, 1 reply; 15+ messages in thread From: Geert Uytterhoeven @ 2011-04-06 20:23 UTC (permalink / raw) To: Jiri Kosina; +Cc: Ingo Molnar, Andrew Morton, linux-kernel, Linux/m68k On Wed, Apr 6, 2011 at 22:08, Jiri Kosina <jkosina@suse.cz> wrote: > On Tue, 29 Mar 2011, Geert Uytterhoeven wrote: > >> I managed to reproduce it inside ARAnyM, by chrooting into the ramdisk image. >> After adding strace, "strace /sbin/init" for the success case gives: [...] >> Seems like the binary doesn't like brk() rounding up the requested >> value to the next page... > > Could you please test with the patch below? Thanks. This seems to work. Thanks! > From: Jiri Kosina <jkosina@suse.cz> > Subject: [PATCH] brk: COMPAT_BRK needs more special handling of legacy applications > > 5520e89 ("brk: fix min_brk lower bound computation for COMPAT_BRK") > tried to get the whole logic of brk randomization for legacy (libc5-based) > applications finally right. > > It turns out that the way to detect whether brk has actually been > randomized or not introduced by that patch still doesn't work for those > binaries, as reported by Geert. > > I don't like it, but currently see no better option than a bit flag in > task_struct to catch the CONFIG_COMPAT_BRK && randomize_va_space == 2 > case. > > Reported-by: Geert Uytterhoeven <geert@linux-m68k.org> > NOT-YET-Signed-off-by: Jiri Kosina <jkosina@suse.cz> > --- > fs/binfmt_elf.c | 6 +++++- > include/linux/sched.h | 3 +++ > mm/mmap.c | 2 +- > 3 files changed, 9 insertions(+), 2 deletions(-) > > diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c > index f34078d..303983f 100644 > --- a/fs/binfmt_elf.c > +++ b/fs/binfmt_elf.c > @@ -941,9 +941,13 @@ 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) && (randomize_va_space > 1)) > + if ((current->flags & PF_RANDOMIZE) && (randomize_va_space > 1)) { > current->mm->brk = current->mm->start_brk = > arch_randomize_brk(current->mm); > +#ifdef CONFIG_COMPAT_BRK > + current->brk_randomized = 1; > +#endif > + } > #endif > > if (current->personality & MMAP_PAGE_ZERO) { > diff --git a/include/linux/sched.h b/include/linux/sched.h > index 83bd2e2..239d2fe 100644 > --- a/include/linux/sched.h > +++ b/include/linux/sched.h > @@ -1254,6 +1254,9 @@ struct task_struct { > #endif > > struct mm_struct *mm, *active_mm; > +#ifdef CONFIG_COMPAT_BRK > + unsigned brk_randomized:1; > +#endif > #if defined(SPLIT_RSS_COUNTING) > struct task_rss_stat rss_stat; > #endif > diff --git a/mm/mmap.c b/mm/mmap.c > index 2ec8eb5..318ed2d 100644 > --- a/mm/mmap.c > +++ b/mm/mmap.c > @@ -259,7 +259,7 @@ SYSCALL_DEFINE1(brk, unsigned long, brk) > * randomize_va_space to 2, which will still cause mm->start_brk > * to be arbitrarily shifted > */ > - if (mm->start_brk > PAGE_ALIGN(mm->end_data)) > + if (current->brk_randomized) > min_brk = mm->start_brk; > else > min_brk = mm->end_data; > -- > 1.7.3.1 Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds ^ permalink raw reply [flat|nested] 15+ messages in thread
* [PATCH] brk: COMPAT_BRK: fix detection of randomized brk (was Re: [regression v2.6.38] Re: [PATCH v2] brk: fix min_brk lower bound computation forCOMPAT_BRK) 2011-04-06 20:23 ` Geert Uytterhoeven @ 2011-04-06 20:38 ` Jiri Kosina 2011-04-06 20:40 ` Geert Uytterhoeven 0 siblings, 1 reply; 15+ messages in thread From: Jiri Kosina @ 2011-04-06 20:38 UTC (permalink / raw) To: Geert Uytterhoeven, Andrew Morton; +Cc: Ingo Molnar, linux-kernel, Linux/m68k From: Jiri Kosina <jkosina@suse.cz> Subject: brk: COMPAT_BRK: fix detection of randomized brk 5520e89 ("brk: fix min_brk lower bound computation for COMPAT_BRK") tried to get the whole logic of brk randomization for legacy (libc5-based) applications finally right. It turns out that the way to detect whether brk has actually been randomized in the end or not introduced by that patch still doesn't work for those binaries, as reported by Geert. I don't like it, but currently see no better option than a bit flag in task_struct to catch the CONFIG_COMPAT_BRK && randomize_va_space == 2 case. Signed-off-by: Jiri Kosina <jkosina@suse.cz> Tested-by: Geert Uytterhoeven <geert@linux-m68k.org> --- I am not really happy about introducing the bit flag, but I currently don't see another option. And it's only for the legacy CONFIG_COMPAT_BRK case anyway. Andrew, Ingo, any opinions/objections? If not -- Andrew, I guess this should go into current -rc still. fs/binfmt_elf.c | 6 +++++- include/linux/sched.h | 3 +++ mm/mmap.c | 2 +- 3 files changed, 9 insertions(+), 2 deletions(-) diff --git a/fs/binfmt_elf.c b/fs/binfmt_elf.c index f34078d..303983f 100644 --- a/fs/binfmt_elf.c +++ b/fs/binfmt_elf.c @@ -941,9 +941,13 @@ 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) && (randomize_va_space > 1)) + if ((current->flags & PF_RANDOMIZE) && (randomize_va_space > 1)) { current->mm->brk = current->mm->start_brk = arch_randomize_brk(current->mm); +#ifdef CONFIG_COMPAT_BRK + current->brk_randomized = 1; +#endif + } #endif if (current->personality & MMAP_PAGE_ZERO) { diff --git a/include/linux/sched.h b/include/linux/sched.h index 83bd2e2..239d2fe 100644 --- a/include/linux/sched.h +++ b/include/linux/sched.h @@ -1254,6 +1254,9 @@ struct task_struct { #endif struct mm_struct *mm, *active_mm; +#ifdef CONFIG_COMPAT_BRK + unsigned brk_randomized:1; +#endif #if defined(SPLIT_RSS_COUNTING) struct task_rss_stat rss_stat; #endif diff --git a/mm/mmap.c b/mm/mmap.c index 2ec8eb5..318ed2d 100644 --- a/mm/mmap.c +++ b/mm/mmap.c @@ -259,7 +259,7 @@ SYSCALL_DEFINE1(brk, unsigned long, brk) * randomize_va_space to 2, which will still cause mm->start_brk * to be arbitrarily shifted */ - if (mm->start_brk > PAGE_ALIGN(mm->end_data)) + if (current->brk_randomized) min_brk = mm->start_brk; else min_brk = mm->end_data; -- 1.7.3.1 ^ permalink raw reply related [flat|nested] 15+ messages in thread
* Re: [PATCH] brk: COMPAT_BRK: fix detection of randomized brk (was Re: [regression v2.6.38] Re: [PATCH v2] brk: fix min_brk lower bound computation forCOMPAT_BRK) 2011-04-06 20:38 ` [PATCH] brk: COMPAT_BRK: fix detection of randomized brk (was Re: [regression v2.6.38] Re: [PATCH v2] brk: fix min_brk lower bound computation forCOMPAT_BRK) Jiri Kosina @ 2011-04-06 20:40 ` Geert Uytterhoeven 2011-04-06 20:42 ` Jiri Kosina 0 siblings, 1 reply; 15+ messages in thread From: Geert Uytterhoeven @ 2011-04-06 20:40 UTC (permalink / raw) To: Jiri Kosina; +Cc: Andrew Morton, Ingo Molnar, linux-kernel, Linux/m68k On Wed, Apr 6, 2011 at 22:38, Jiri Kosina <jkosina@suse.cz> wrote: > From: Jiri Kosina <jkosina@suse.cz> > Subject: brk: COMPAT_BRK: fix detection of randomized brk > > 5520e89 ("brk: fix min_brk lower bound computation for COMPAT_BRK") > tried to get the whole logic of brk randomization for legacy (libc5-based) > applications finally right. > > It turns out that the way to detect whether brk has actually been randomized in > the end or not introduced by that patch still doesn't work for those binaries, > as reported by Geert. > > I don't like it, but currently see no better option than a bit flag in > task_struct to catch the CONFIG_COMPAT_BRK && randomize_va_space == 2 > case. > > Signed-off-by: Jiri Kosina <jkosina@suse.cz> > Tested-by: Geert Uytterhoeven <geert@linux-m68k.org> > --- > > I am not really happy about introducing the bit flag, but I currently > don't see another option. And it's only for the legacy CONFIG_COMPAT_BRK > case anyway. > > Andrew, Ingo, any opinions/objections? > > If not -- Andrew, I guess this should go into current -rc still. And in 2.6.38-stable. Does anyone still have libc5 binaries for i386? Gr{oetje,eeting}s, Geert -- Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@linux-m68k.org In personal conversations with technical people, I call myself a hacker. But when I'm talking to journalists I just say "programmer" or something like that. -- Linus Torvalds ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [PATCH] brk: COMPAT_BRK: fix detection of randomized brk (was Re: [regression v2.6.38] Re: [PATCH v2] brk: fix min_brk lower bound computation forCOMPAT_BRK) 2011-04-06 20:40 ` Geert Uytterhoeven @ 2011-04-06 20:42 ` Jiri Kosina 0 siblings, 0 replies; 15+ messages in thread From: Jiri Kosina @ 2011-04-06 20:42 UTC (permalink / raw) To: Geert Uytterhoeven; +Cc: Andrew Morton, Ingo Molnar, linux-kernel, Linux/m68k On Wed, 6 Apr 2011, Geert Uytterhoeven wrote: > > From: Jiri Kosina <jkosina@suse.cz> > > Subject: brk: COMPAT_BRK: fix detection of randomized brk > > > > 5520e89 ("brk: fix min_brk lower bound computation for COMPAT_BRK") > > tried to get the whole logic of brk randomization for legacy (libc5-based) > > applications finally right. > > > > It turns out that the way to detect whether brk has actually been randomized in > > the end or not introduced by that patch still doesn't work for those binaries, > > as reported by Geert. > > > > I don't like it, but currently see no better option than a bit flag in > > task_struct to catch the CONFIG_COMPAT_BRK && randomize_va_space == 2 > > case. > > > > Signed-off-by: Jiri Kosina <jkosina@suse.cz> > > Tested-by: Geert Uytterhoeven <geert@linux-m68k.org> > > --- > > > > I am not really happy about introducing the bit flag, but I currently > > don't see another option. And it's only for the legacy CONFIG_COMPAT_BRK > > case anyway. > > > > Andrew, Ingo, any opinions/objections? > > > > If not -- Andrew, I guess this should go into current -rc still. > > And in 2.6.38-stable. > > Does anyone still have libc5 binaries for i386? The first time we introduced brk randomization, Pavel Machek reported a problem with some libc5-based binary on some ancient x86 system he had. So I am afraid there still might be sparse occurences out there. -- Jiri Kosina SUSE Labs, Novell Inc. ^ permalink raw reply [flat|nested] 15+ messages in thread
* Re: [regression v2.6.38] Re: [PATCH v2] brk: fix min_brk lower bound computation for COMPAT_BRK 2011-03-26 13:51 ` Geert Uytterhoeven 2011-03-28 14:20 ` Jiri Kosina @ 2011-03-29 12:02 ` Jiri Kosina 1 sibling, 0 replies; 15+ messages in thread From: Jiri Kosina @ 2011-03-29 12:02 UTC (permalink / raw) To: Geert Uytterhoeven; +Cc: Ingo Molnar, Andrew Morton, linux-kernel, Linux/m68k On Sat, 26 Mar 2011, Geert Uytterhoeven wrote: > >> > From: Jiri Kosina <jkosina@suse.cz> > >> > Subject: [PATCH] brk: fix min_brk lower bound computation for COMPAT_BRK > >> > > >> > Even if CONFIG_COMPAT_BRK is set in the kernel configuration, it can still > >> > be overriden by randomize_va_space sysctl. > >> > > >> > If this is the case, the min_brk computation in sys_brk() implementation > >> > is wrong, as it solely takes into account COMPAT_BRK setting, assuming > >> > that brk start is not randomized. But that might not be the case if > >> > randomize_va_space sysctl has been set to '2' at the time the binary has > >> > been loaded from disk. > >> > > >> > In such case, the check has to be done in a same way as in > >> > !CONFIG_COMPAT_BRK case. > >> > > >> > In addition to that, the check for the COMPAT_BRK case introduced back in > >> > a5b4592c ("brk: make sys_brk() honor COMPAT_BRK when computing lower > >> > bound") is slightly wrong -- the lower bound shouldn't be mm->end_code, > >> > but mm->end_data instead, as that's where the legacy applications expect > >> > brk section to start (i.e. immediately after last global variable). > >> > > >> > Signed-off-by: Jiri Kosina <jkosina@suse.cz> > >> > --- > >> > mm/mmap.c | 10 +++++++++- > >> > 1 files changed, 9 insertions(+), 1 deletions(-) > >> > > >> > diff --git a/mm/mmap.c b/mm/mmap.c > >> > index 50a4aa0..ca2f164 100644 > >> > --- a/mm/mmap.c > >> > +++ b/mm/mmap.c > >> > @@ -253,7 +253,15 @@ SYSCALL_DEFINE1(brk, unsigned long, brk) > >> > down_write(&mm->mmap_sem); > >> > > >> > #ifdef CONFIG_COMPAT_BRK > >> > - min_brk = mm->end_code; > >> > + /* > >> > + * CONFIG_COMPAT_BRK can still be overridden by setting > >> > + * randomize_va_space to 2, which will still make mm->start_brk > >> > + * to be arbitrarily shifted > >> > + */ > >> > + if (mm->start_brk > PAGE_ALIGN(mm->end_data)) > >> > + min_brk = mm->start_brk; > >> > + else > >> > + min_brk = mm->end_data; > >> > #else > >> > min_brk = mm->start_brk; > >> > #endif > >> > -- > >> > 1.7.3.1 > >> > >> Sorry for chiming in this late, but I've just bisected a problem in > >> 2.6.38 to commit > >> 5520e89485252c759ee60d313e9422447659947b ("brk: fix min_brk lower bound > >> computation for COMPAT_BRK"). > >> > >> When booting my very old test ramdisk on Amiga/m68k, it fails like this: > >> > >> | RAMDISK: gzip image found at block 0 > >> | VFS: Mounted root (ext2 filesystem) readonly on device 1:0. > >> | warning: process `update' used the obsolete bdflush system call > >> | Fix your initscripts? > >> | init: cannot open inittab > >> | Kernel panic - not syncing: Attempted to kill init! > >> > >> Sorry for not noticing earlier, I usually boot full Debians under ARAnyM, > >> instead of booting old ramdisks with libc5-based binaries that once were > >> considered new. > > > > Oh well, one has to love the libc5-based binaries indeed. > > Yeah, binaries from 1996 ;-) > > > Is the patch below fixing the issue you are seeing on your Amiga/m68k? > > Thanks. > > > > > > diff --git a/mm/mmap.c b/mm/mmap.c > > index 2ec8eb5..0a02531 100644 > > --- a/mm/mmap.c > > +++ b/mm/mmap.c > > @@ -262,7 +262,7 @@ SYSCALL_DEFINE1(brk, unsigned long, brk) > > if (mm->start_brk > PAGE_ALIGN(mm->end_data)) > > min_brk = mm->start_brk; > > else > > - min_brk = mm->end_data; > > + min_brk = mm->end_code; > > #else > > min_brk = mm->start_brk; > > #endif > > Unfortunately not... > > I added some printk()s: > > mm->start_brk = 0x8000a000, PAGE_ALIGN(mm->end_data = 0x8000a000) > mm->start_brk = 0x8000a000, PAGE_ALIGN(mm->end_data = 0x8000a000) > mm->start_brk = 0x8000a000, PAGE_ALIGN(mm->end_data = 0x8000a000) > mm->start_brk = 0x8000a000, PAGE_ALIGN(mm->end_data = 0x8000a000) > mm->start_brk = 0x80006000, PAGE_ALIGN(mm->end_data = 0x80004000) > > I.e. just before the failure, "mm->start_brk > PAGE_ALIGN(mm->end_data)" > became true. Actually I believe that this is a different binary being executed -- the first 4 lines correspond to some other binary being loaded (which works fine). The whole point of if (mm->start_brk > PAGE_ALIGN(mm->end_data)) is to decide whether start_brk has been randomized even though COMPAT_BRK has been set (forcing it by randomize_va_space == 2). But apparently even this doesn't work in your ancienc-libc5-static(?) library case, as start_brk is one page off anyway. So I am a little bit clueless how to do the detection properly (we definitely can't account this one extra page as that will break other cases). And I really wouldn't like to add MMF_BRK_RANDOMIZED flag to mm_struct->flags just for this very legacy case. So any ideas are welcome. The strace of the failing binary would still be helpful. Thanks, -- Jiri Kosina SUSE Labs, Novell Inc. ^ permalink raw reply [flat|nested] 15+ messages in thread
end of thread, other threads:[~2011-04-06 20:42 UTC | newest] Thread overview: 15+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2011-01-02 23:05 [PATCH] brk: fix min_brk lower bound computation for COMPAT_BRK Jiri Kosina 2011-01-03 11:39 ` Jiri Kosina 2011-01-03 13:24 ` [PATCH v2] " Jiri Kosina 2011-03-24 21:49 ` [regression v2.6.38] " Geert Uytterhoeven 2011-03-25 10:20 ` Jiri Kosina 2011-03-26 13:51 ` Geert Uytterhoeven 2011-03-28 14:20 ` Jiri Kosina 2011-03-29 20:24 ` Geert Uytterhoeven 2011-03-29 20:37 ` Andreas Schwab 2011-04-06 20:08 ` Jiri Kosina 2011-04-06 20:23 ` Geert Uytterhoeven 2011-04-06 20:38 ` [PATCH] brk: COMPAT_BRK: fix detection of randomized brk (was Re: [regression v2.6.38] Re: [PATCH v2] brk: fix min_brk lower bound computation forCOMPAT_BRK) Jiri Kosina 2011-04-06 20:40 ` Geert Uytterhoeven 2011-04-06 20:42 ` Jiri Kosina 2011-03-29 12:02 ` [regression v2.6.38] Re: [PATCH v2] brk: fix min_brk lower bound computation for COMPAT_BRK Jiri Kosina
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox