| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Message-ID: <alpine.LNX.2.00.1103291358440.26754@pobox.suse.cz>
Date: Tue, 29 Mar 2011 14:02:06 +0200 (CEST)
From: Jiri Kosina <jkosina@...e.cz>
To: Geert Uytterhoeven <geert@...ux-m68k.org>
Cc: Ingo Molnar <mingo@...e.hu>,
Andrew Morton <akpm@...ux-foundation.org>,
linux-kernel@...r.kernel.org,
Linux/m68k <linux-m68k@...r.kernel.org>
Subject: Re: [regression v2.6.38] Re: [PATCH v2] brk: fix min_brk lower bound
computation for COMPAT_BRK
On Sat, 26 Mar 2011, Geert Uytterhoeven wrote:
> >> > From: Jiri Kosina <jkosina@...e.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@...e.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.
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists