linux-kernel - Re: [regression v2.6.38] Re: [PATCH v2] brk: fix min_brk lower bound computation for COMPAT

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

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <AANLkTi=g5z+GYkbOGbdLQcLaQkpY6yQ6tjGRjjk1H59R@mail.gmail.com>
Date:	Sat, 26 Mar 2011 14:51:49 +0100
From:	Geert Uytterhoeven <geert@...ux-m68k.org>
To:	Jiri Kosina <jkosina@...e.cz>
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

Hi Jiri,

On Fri, Mar 25, 2011 at 11:20, Jiri Kosina <jkosina@...e.cz> wrote:
> On Thu, 24 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.

Gr{oetje,eeting}s,

                        Geert

--
Geert Uytterhoeven -- There's lots of Linux beyond ia32 -- geert@...ux-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
--
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/