> > > note: it's untested.
> >
> > Works for me on ppc64 with 4k and 64k pages. Thanks!
> >
> > 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.
>
> Guys, here's the rolled-up patch.
FYI the rolled up patch still works fine on PPC64. Thanks.
> Could someone please test it on parisc?
>
> err, I'm not sure what one needs to do to test it, actually.
> Presumably it involves setting an unusual `ulimit -s'. Can someone
> please suggest a test plan?
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 trickier
case to handle correctly with this code.
4K pages on parisc should be fine to test with.
Mikey
>
> From: Michael Neuling <mikey@xxxxxxxxxxx>
>
> When reserving stack space for a new process, make sure we're not
> attempting to expand the stack by more than rlimit allows.
>
> This fixes a bug caused by b6a2fea39318e43fee84fa7b0b90d68bed92d2ba ("mm:
> variable length argument support") and unmasked by
> fc63cf237078c86214abcb2ee9926d8ad289da9b ("exec: setup_arg_pages() fails
> to return errors").
>
> 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.
>
> Signed-off-by: Michael Neuling <mikey@xxxxxxxxxxx>
> Cc: KOSAKI Motohiro <kosaki.motohiro@xxxxxxxxxxxxxx>
> Cc: Americo Wang <xiyou.wangcong@xxxxxxxxx>
> Cc: Anton Blanchard <anton@xxxxxxxxx>
> Cc: Oleg Nesterov <oleg@xxxxxxxxxx>
> Cc: James Morris <jmorris@xxxxxxxxx>
> Cc: Ingo Molnar <mingo@xxxxxxx>
> Cc: Serge Hallyn <serue@xxxxxxxxxx>
> Cc: Benjamin Herrenschmidt <benh@xxxxxxxxxxxxxxxxxxx>
> Cc: <stable@xxxxxxxxxx>
>
> fs/exec.c | 21 +++++++++++++++++++--
> 1 file changed, 19 insertions(+), 2 deletions(-)
>
> diff -puN fs/exec.c~fs-execc-restrict-initial-stack-space-expansion-to-rlimit
fs/exec.c
> --- a/fs/exec.c~fs-execc-restrict-initial-stack-space-expansion-to-rlimit
> +++ a/fs/exec.c
> @@ -571,6 +571,9 @@ int setup_arg_pages(struct linux_binprm
> struct vm_area_struct *prev = NULL;
> unsigned long vm_flags;
> unsigned long stack_base;
> + unsigned long stack_size;
> + unsigned long stack_expand;
> + unsigned long rlim_stack;
>
> #ifdef CONFIG_STACK_GROWSUP
> /* Limit stack size to 1GB */
> @@ -627,10 +630,24 @@ int setup_arg_pages(struct linux_binprm
> goto out_unlock;
> }
>
> + stack_expand = EXTRA_STACK_VM_PAGES * PAGE_SIZE;
> + stack_size = vma->vm_end - vma->vm_start;
> + /*
> + * Align this down to a page boundary as expand_stack
> + * will align it up.
> + */
> + rlim_stack = rlimit(RLIMIT_STACK) & PAGE_MASK;
> + rlim_stack = min(rlim_stack, stack_size);
> #ifdef CONFIG_STACK_GROWSUP
> - stack_base = vma->vm_end + EXTRA_STACK_VM_PAGES * PAGE_SIZE;
> + if (stack_size + stack_expand > rlim_stack)
> + stack_base = vma->vm_start + rlim_stack;
> + else
> + stack_base = vma->vm_end + stack_expand;
> #else
> - stack_base = vma->vm_start - EXTRA_STACK_VM_PAGES * PAGE_SIZE;
> + if (stack_size + stack_expand > rlim_stack)
> + stack_base = vma->vm_end - rlim_stack;
> + else
> + stack_base = vma->vm_start - stack_expand;
> #endif
> ret = expand_stack(vma, stack_base);
> if (ret)
> _
>
--
To unsubscribe from this list: send the line "unsubscribe linux-fsdevel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
