[BUGFIX][PATCH] fixing placement of register stack under ulimit -s

[BUGFIX][PATCH] fixing placement of register stack under ulimit -s

[KAMEZAWA Hiroyuki]

This patch fixes ia64's bug in ulimit -s handling. against 2.6.21-rc3.

At first,the address of register stack is defined by this
== ia64_set_rbs_bot()
unsigned long stack_size = current->signal->rlim[RLIMIT_STACK].rlim_max & -16;

        if (stack_size > MAX_USER_STACK_SIZE)
                stack_size = MAX_USER_STACK_SIZE;
        current->thread.rbs_bot = STACK_TOP - stack_size;
}
==
default rlim[RLIMIT_STACK].rlim_max is very big.

If ulimit -s is used, rlim_max can be small. As a result, rbs_bot can be
very high address. Because of stack-address-randomize, there is a case that
"regiter stack" is placed in the higher address than "memory stack".
== example...
[kamezawa@drpq linux-2.6.21-rc2]$ ulimit -s
8192
[kamezawa@drpq linux-2.6.21-rc2]$ cat /proc/self/maps
00000000-00004000 r--p 00000000 00:00 0
2000000000000000-2000000000038000 r-xp 00000000 08:02 5991525            /lib/ld-2.5.so
2000000000044000-2000000000050000 rw-p 00034000 08:02 5991525            /lib/ld-2.5.so
2000000000064000-20000000002c8000 r-xp 00000000 08:02 5990699            /lib/libc-2.5.so
20000000002c8000-20000000002d4000 ---p 00264000 08:02 5990699            /lib/libc-2.5.so
20000000002d4000-20000000002e0000 rw-p 00260000 08:02 5990699            /lib/libc-2.5.so
20000000002e0000-20000000002ec000 rw-p 20000000002e0000 00:00 0
20000000002ec000-2000000003a24000 r--p 00000000 08:02 9472842            /usr/lib/locale/locale-archive
4000000000000000-4000000000008000 r-xp 00000000 08:02 4157490            /bin/cat
6000000000004000-600000000000c000 rw-p 00004000 08:02 4157490            /bin/cat
600000000000c000-6000000000030000 rw-p 600000000000c000 00:00 0          [heap]
60000fffff5f4000-60000fffff648000 rw-p 60000fffff5f4000 00:00 0          [stack]
60000fffff7fc000-60000fffff800000 rw-p 60000fffff7fc000 00:00 0          (*)
a000000000000000-a000000000020000 ---p 00000000 00:00 0                  [vdso]
(*) is register stack.
==
This register-stack/memory-stack upside down is not expected.
Current ia64 page fault handler doesn't handle this case. In this case, 
register stack expansion causes SEGV.
This means that the user program can use only 1 page for its register stack.

This patch fixes the above case by moving register stack to suitable place.
Note) fixing page fault handler seems to be another way...but a bit complicated.

Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
---
Index: linux-2.6.21-rc3/arch/ia64/mm/init.c
===================================================================
--- linux-2.6.21-rc3.orig/arch/ia64/mm/init.c
+++ linux-2.6.21-rc3/arch/ia64/mm/init.c
@@ -155,7 +155,7 @@ ia64_set_rbs_bot (void)
 
 	if (stack_size > MAX_USER_STACK_SIZE)
 		stack_size = MAX_USER_STACK_SIZE;
-	current->thread.rbs_bot = STACK_TOP - stack_size;
+	current->thread.rbs_bot = PAGE_ALIGN(current->mm->start_stack - stack_size);
 }
 
 /*

Re: [BUGFIX][PATCH] fixing placement of register stack under ulimit -s

[David Mosberger-Tang]

But aren't you going to be limited to less than a page worth of
register-backing store even with your patch applied because the
backing store will end up overflowing the memory stack?

  --david

On 3/15/07, KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> wrote:
> This patch fixes ia64's bug in ulimit -s handling. against 2.6.21-rc3.
>
> At first,the address of register stack is defined by this
> == ia64_set_rbs_bot()
> unsigned long stack_size = current->signal->rlim[RLIMIT_STACK].rlim_max & -16;
>
>         if (stack_size > MAX_USER_STACK_SIZE)
>                 stack_size = MAX_USER_STACK_SIZE;
>         current->thread.rbs_bot = STACK_TOP - stack_size;
> }
> ==
> default rlim[RLIMIT_STACK].rlim_max is very big.
>
> If ulimit -s is used, rlim_max can be small. As a result, rbs_bot can be
> very high address. Because of stack-address-randomize, there is a case that
> "regiter stack" is placed in the higher address than "memory stack".
> == example...
> [kamezawa@drpq linux-2.6.21-rc2]$ ulimit -s
> 8192
> [kamezawa@drpq linux-2.6.21-rc2]$ cat /proc/self/maps
> 00000000-00004000 r--p 00000000 00:00 0
> 2000000000000000-2000000000038000 r-xp 00000000 08:02 5991525            /lib/ld-2.5.so
> 2000000000044000-2000000000050000 rw-p 00034000 08:02 5991525            /lib/ld-2.5.so
> 2000000000064000-20000000002c8000 r-xp 00000000 08:02 5990699            /lib/libc-2.5.so
> 20000000002c8000-20000000002d4000 ---p 00264000 08:02 5990699            /lib/libc-2.5.so
> 20000000002d4000-20000000002e0000 rw-p 00260000 08:02 5990699            /lib/libc-2.5.so
> 20000000002e0000-20000000002ec000 rw-p 20000000002e0000 00:00 0
> 20000000002ec000-2000000003a24000 r--p 00000000 08:02 9472842            /usr/lib/locale/locale-archive
> 4000000000000000-4000000000008000 r-xp 00000000 08:02 4157490            /bin/cat
> 6000000000004000-600000000000c000 rw-p 00004000 08:02 4157490            /bin/cat
> 600000000000c000-6000000000030000 rw-p 600000000000c000 00:00 0          [heap]
> 60000fffff5f4000-60000fffff648000 rw-p 60000fffff5f4000 00:00 0          [stack]
> 60000fffff7fc000-60000fffff800000 rw-p 60000fffff7fc000 00:00 0          (*)
> a000000000000000-a000000000020000 ---p 00000000 00:00 0                  [vdso]
> (*) is register stack.
> ==
> This register-stack/memory-stack upside down is not expected.
> Current ia64 page fault handler doesn't handle this case. In this case,
> register stack expansion causes SEGV.
> This means that the user program can use only 1 page for its register stack.
>
> This patch fixes the above case by moving register stack to suitable place.
> Note) fixing page fault handler seems to be another way...but a bit complicated.
>
> Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> ---
> Index: linux-2.6.21-rc3/arch/ia64/mm/init.c
> ===================================================================
> --- linux-2.6.21-rc3.orig/arch/ia64/mm/init.c
> +++ linux-2.6.21-rc3/arch/ia64/mm/init.c
> @@ -155,7 +155,7 @@ ia64_set_rbs_bot (void)
>
>         if (stack_size > MAX_USER_STACK_SIZE)
>                 stack_size = MAX_USER_STACK_SIZE;
> -       current->thread.rbs_bot = STACK_TOP - stack_size;
> +       current->thread.rbs_bot = PAGE_ALIGN(current->mm->start_stack - stack_size);
>  }
>
>  /*
>
> -
> To unsubscribe from this list: send the line "unsubscribe linux-ia64" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>


-- 
Mosberger Consulting LLC, http://www.mosberger-consulting.com/

Re: [BUGFIX][PATCH] fixing placement of register stack under ulimit -s

[KAMEZAWA Hiroyuki]

On Thu, 15 Mar 2007 09:57:28 -0600
"David Mosberger-Tang" <dmosberger@gmail.com> wrote:

> But aren't you going to be limited to less than a page worth of
> register-backing store even with your patch applied because the
> backing store will end up overflowing the memory stack?
> 

I think pthread's stack, which is created by malloc, is also shared
among register-stack and memory-stack. 
(glibc's pthread's stack is limited by ulimit, too.)

So, it seems stack_size_limit = register_stack_limit + memory_stack_limit
is a consistent way. I'm sorry if I don't catch your point.

 --Kame


>   --david
> 
> On 3/15/07, KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> wrote:
> > This patch fixes ia64's bug in ulimit -s handling. against 2.6.21-rc3.
> >
> > At first,the address of register stack is defined by this
> > == ia64_set_rbs_bot()
> > unsigned long stack_size = current->signal->rlim[RLIMIT_STACK].rlim_max & -16;
> >
> >         if (stack_size > MAX_USER_STACK_SIZE)
> >                 stack_size = MAX_USER_STACK_SIZE;
> >         current->thread.rbs_bot = STACK_TOP - stack_size;
> > }
> > ==
> > default rlim[RLIMIT_STACK].rlim_max is very big.
> >
> > If ulimit -s is used, rlim_max can be small. As a result, rbs_bot can be
> > very high address. Because of stack-address-randomize, there is a case that
> > "regiter stack" is placed in the higher address than "memory stack".
> > == example...
> > [kamezawa@drpq linux-2.6.21-rc2]$ ulimit -s
> > 8192
> > [kamezawa@drpq linux-2.6.21-rc2]$ cat /proc/self/maps
> > 00000000-00004000 r--p 00000000 00:00 0
> > 2000000000000000-2000000000038000 r-xp 00000000 08:02 5991525            /lib/ld-2.5.so
> > 2000000000044000-2000000000050000 rw-p 00034000 08:02 5991525            /lib/ld-2.5.so
> > 2000000000064000-20000000002c8000 r-xp 00000000 08:02 5990699            /lib/libc-2.5.so
> > 20000000002c8000-20000000002d4000 ---p 00264000 08:02 5990699            /lib/libc-2.5.so
> > 20000000002d4000-20000000002e0000 rw-p 00260000 08:02 5990699            /lib/libc-2.5.so
> > 20000000002e0000-20000000002ec000 rw-p 20000000002e0000 00:00 0
> > 20000000002ec000-2000000003a24000 r--p 00000000 08:02 9472842            /usr/lib/locale/locale-archive
> > 4000000000000000-4000000000008000 r-xp 00000000 08:02 4157490            /bin/cat
> > 6000000000004000-600000000000c000 rw-p 00004000 08:02 4157490            /bin/cat
> > 600000000000c000-6000000000030000 rw-p 600000000000c000 00:00 0          [heap]
> > 60000fffff5f4000-60000fffff648000 rw-p 60000fffff5f4000 00:00 0          [stack]
> > 60000fffff7fc000-60000fffff800000 rw-p 60000fffff7fc000 00:00 0          (*)
> > a000000000000000-a000000000020000 ---p 00000000 00:00 0                  [vdso]
> > (*) is register stack.
> > ==
> > This register-stack/memory-stack upside down is not expected.
> > Current ia64 page fault handler doesn't handle this case. In this case,
> > register stack expansion causes SEGV.
> > This means that the user program can use only 1 page for its register stack.
> >
> > This patch fixes the above case by moving register stack to suitable place.
> > Note) fixing page fault handler seems to be another way...but a bit complicated.
> >
> > Signed-off-by: KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com>
> > ---
> > Index: linux-2.6.21-rc3/arch/ia64/mm/init.c
> > ===================================================================
> > --- linux-2.6.21-rc3.orig/arch/ia64/mm/init.c
> > +++ linux-2.6.21-rc3/arch/ia64/mm/init.c
> > @@ -155,7 +155,7 @@ ia64_set_rbs_bot (void)
> >
> >         if (stack_size > MAX_USER_STACK_SIZE)
> >                 stack_size = MAX_USER_STACK_SIZE;
> > -       current->thread.rbs_bot = STACK_TOP - stack_size;
> > +       current->thread.rbs_bot = PAGE_ALIGN(current->mm->start_stack - stack_size);
> >  }
> >
> >  /*
> >
> > -
> > To unsubscribe from this list: send the line "unsubscribe linux-ia64" in
> > the body of a message to majordomo@vger.kernel.org
> > More majordomo info at  http://vger.kernel.org/majordomo-info.html
> >
> 
> 
> -- 
> Mosberger Consulting LLC, http://www.mosberger-consulting.com/
> -
> To unsubscribe from this list: send the line "unsubscribe linux-ia64" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [BUGFIX][PATCH] fixing placement of register stack under ulimit -s

[KAMEZAWA Hiroyuki]

On Fri, 16 Mar 2007 06:20:47 +0900
KAMEZAWA Hiroyuki <kamezawa.hiroyu@jp.fujitsu.com> wrote:

> On Thu, 15 Mar 2007 09:57:28 -0600
> "David Mosberger-Tang" <dmosberger@gmail.com> wrote:
> 
> > But aren't you going to be limited to less than a page worth of
> > register-backing store even with your patch applied because the
> > backing store will end up overflowing the memory stack?
> > 
> 
> I think pthread's stack, which is created by malloc, is also shared
> among register-stack and memory-stack. 
> (glibc's pthread's stack is limited by ulimit, too.)
> 
> So, it seems stack_size_limit = register_stack_limit + memory_stack_limit
> is a consistent way. I'm sorry if I don't catch your point.
> 
BTW, what way do you recommened to fix this register-stack/memory-stack upside
down problem ?

Plan A) just handle upside-down case in page fault handler.
	This means ulimit -s limitation will limit amount of memory-stack and
	register stack independently.
Plan B) handle upside-down case in page fault handler and add modify 
 	acct_stack_growth() to be able to handle the limitation of sum
	of separated vmas.(vma for reg stack and mem stack). 
Plan C) don't allow this upside down as this patch. but change calculation of rbs_top.

Note:
To see the problem which my patch want to fix run following code under ulimit -s.

==
void eat_stack(int num) {
	printf("%d\n", num);
	eat_stack(num - 1);
}

int main (void) {
	eat_stack(10000);
}
==

-- Kame

Re: [BUGFIX][PATCH] fixing placement of register stack under ulimit -s

[KAMEZAWA Hiroyuki]

plz allow me to explain more.

"Why register-stack/memory-stack upside down is bad" is a bit complicated.
So...this is a test and result for explaining bug. 

This is a sample code and its result on 2.6.21-rc3.
Note: base address of memory'stack can be randomly change.

== sample code ==
[kamezawa@drpq ~]$ cat sample.c
#include <stdio.h>

void do_print(int num)
{
        if (num == 0)
                return;
        printf("%d\n",num);
        do_print(num - 1);
}

int main(int argc, char *argv[])
{
        do_print(10000);
        return 0;
}

== before ulimit ==
[kamezawa@drpq ~]$ uname -a
Linux drpq 2.6.21-rc3 #3 SMP Fri Mar 16 11:57:47 JST 2007 ia64 ia64 ia64 GNU/Linux
[kamezawa@drpq ~]$ ulimit -s
8192
[kamezawa@drpq ~]$ ulimit -s -H
unlimited
[kamezawa@drpq ~]$ ./sample
10000
9999
....<snip>
1
[kamezawa@drpq ~]$
== after ulimit -s 8192 ==

[kamezawa@drpq ~]$ ulimit -s
8192
[kamezawa@drpq ~]$ ulimit -s -H
8192
[kamezawa@drpq ~]$ ./sample  
10000
9999
....<snip>

9612
9611
9610
9609
9608
Segmentation fault

[kamezawa@drpq ~]$ ./sample   (when I'm lucky)
10000
9999
....<snip>
1
[kamezawa@drpq ~]$
=====================================

This number 9608 is too short to use up all stack. The reason of this is 
"ulimit -s + memory stack randomization + register-stack-expansion" is buggy.
The program can only use one page for register stack if unlucky.
My patch will fix this case.

-Kame