Re: [PATCH] Restrict initial stack space expansion to rlimit

In message [ref] you wrote:

On 02/10/2010 06:31 AM, Michael Neuling wrote:

quoted

In message[ref]  you wrote:

quoted

quoted

On 02/09/2010 10:51 PM, Michael Neuling wrote:

quoted

quoted

quoted

I'd still like someone with a CONFIG_STACK_GROWSUP arch to test/ACK it
as well.

There's only one CONFIG_GROWSUP arch - parisc.
Could someone please test it on parisc?

I did.

quoted

How about doing:
    'ulimit -s 15; ls'
before and after the patch is applied.  Before it's applied, 'ls' should
be killed.  After the patch is applied, 'ls' should no longer be killed.

I'm suggesting a stack limit of 15KB since it's small enough to trigger
20*PAGE_SIZE.  Also 15KB not a multiple of PAGE_SIZE, which is a trickie

r

quoted

quoted

quoted

quoted

case to handle correctly with this code.

4K pages on parisc should be fine to test with.

Mikey, thanks for the suggested test plan.

I'm not sure if your patch does it correct for parisc/stack-grows-up-case

.

quoted

quoted

quoted

I tested your patch on  a 4k pages kernel:
root@c3000:~# uname -a
Linux c3000 2.6.33-rc7-32bit #221 Tue Feb 9 23:17:06 CET 2010 parisc GNU/

Li

quoted

nux

quoted

quoted

Without your patch:
root@c3000:~# ulimit -s 15; ls
Killed
->  correct.

With your patch:
root@c3000:~# ulimit -s 15; ls
Killed
_or_:
root@c3000:~# ulimit -s 15; ls
Segmentation fault
->  ??

Any idea?

My x86_64 box also makes segmentation fault. I think "ulimit -s 15" is too

 sm

quoted

all stack for ls.

quoted

"ulimit -s 27; ls "  wroks perfectly fine.

Arrh.  I asked Helge offline earlier to check what use to work on parisc
on 2.6.31.

I guess PPC has a nice clean non-bloated ABI :-D

Hi Mikey,

I tested again, and it works for me with "ulimit -s 27" as well (on a
4k, 32bit kernel).
Still, I'm not 100%  sure if your patch is correct.

Thanks for retesting

Did "ulimit -s 27" fail before you applied?

Anyway, it seems to work.

But what makes me wonder is, why EXTRA_STACK_VM_PAGES is defined in pages at 

all.

You wrote in your patch description:

quoted

This bug means that when limiting the stack to less the 20*PAGE_SIZE (eg.
80K on 4K pages or 'ulimit -s 79') all processes will be killed before
they start.  This is particularly bad with 64K pages, where a ulimit below
1280K will kill every process.

Wouldn't it make sense to define and use EXTRA_STACK_VM_SIZE instead
(e.g. as 20*4096 = 80k)?  This extra stack reservation should IMHO be
independend of the actual kernel page size.

If you look back through this thread, that has already been noted but
it's a separate issue to this bug, so that change will be deferred till
2.6.34.

Mikey