public inbox for linux-kernel@vger.kernel.org
 help / color / mirror / Atom feed
* [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-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

* 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

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