Re: [PATCH] Restrict stack space reservation to rlimit
In message 20100208145240.fb58.a69d9...@jp.fujitsu.com you wrote: Hi, Why do we need page size independent stack size? It seems to have compatibility breaking risk. I don't think so. The current behaviour is clearly wrong, we dont need a 16x larger stack just because you went from a 4kB to a 64kB base page size. The user application stack usage is the same in both cases. I didn't discuss which behavior is better. Michael said he want to apply his patch to 2.6.32 2.6.33. stable tree never accept the breaking compatibility patch. Your answer doesn't explain why can't we wait it until next merge window. btw, personally, I like page size indepent stack size. but I'm not sure why making stack size independency is related to bug fix. I tend to agree. Below is just the bug fix to limit the reservation size based rlimit. We still reserve different stack sizes based on the page size as before (unless we hit rlimit of course). Thanks. I agree your patch in almost part. but I have very few requests. Mikey Restrict stack space reservation to rlimit When reserving stack space for a new process, make sure we're not attempting to allocate more than rlimit allows. This fixes a bug cause by b6a2fea39318e43fee84fa7b0b90d68bed92d2ba mm: variable length argument support and unmasked by fc63cf237078c86214abcb2ee9926d8ad289da9b exec: setup_arg_pages() fails to return errors. Your initial mail have following problem use-case. please append it into the patch description. On recent ppc64 kernels, limiting the stack (using 'ulimit -s blah') is now more restrictive than it was before. On 2.6.31 with 4k pages I could run 'ulimit -s 16; /usr/bin/test' without a problem. Now with mainline, even 'ulimit -s 64; /usr/bin/test' gets killed. Ok, I'll add this info in. Signed-off-by: Michael Neuling mi...@neuling.org Cc: Anton Blanchard an...@samba.org Cc: sta...@kernel.org --- fs/exec.c |7 +-- 1 file changed, 5 insertions(+), 2 deletions(-) Index: linux-2.6-ozlabs/fs/exec.c === --- linux-2.6-ozlabs.orig/fs/exec.c +++ linux-2.6-ozlabs/fs/exec.c @@ -627,10 +627,13 @@ int setup_arg_pages(struct linux_binprm goto out_unlock; } + stack_base = min(EXTRA_STACK_VM_PAGES * PAGE_SIZE, +current-signal-rlim[RLIMIT_STACK].rlim_cur - + PAGE_SIZE); This line is a bit unclear why - PAGE_SIZE is necessary. This is because the stack is already 1 page in size. I'm going to change that code to make it clearer... hopefully :-) personally, I like following likes explicit comments. stack_expand = EXTRA_STACK_VM_PAGES * PAGE_SIZE; stack_lim = ACCESS_ONCE(rlim[RLIMIT_STACK].rlim_cur); /* Initial stack must not cause stack overflow. */ if (stack_expand + PAGE_SIZE stack_lim) stack_expand = stack_lim - PAGE_SIZE; note: accessing rlim_cur require ACCESS_ONCE. Thought? Thanks, looks better/clearer to me too. I'll change, new patch coming Mikey #ifdef CONFIG_STACK_GROWSUP - stack_base = vma-vm_end + EXTRA_STACK_VM_PAGES * PAGE_SIZE; + stack_base = vma-vm_end + stack_base; #else - stack_base = vma-vm_start - EXTRA_STACK_VM_PAGES * PAGE_SIZE; + stack_base = vma-vm_start - stack_base; #endif ret = expand_stack(vma, stack_base); if (ret) ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] Restrict stack space reservation to rlimit
Hi, Why do we need page size independent stack size? It seems to have compatibility breaking risk. I don't think so. The current behaviour is clearly wrong, we dont need a 16x larger stack just because you went from a 4kB to a 64kB base page size. The user application stack usage is the same in both cases. Anton ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] Restrict stack space reservation to rlimit
Hi, Why do we need page size independent stack size? It seems to have compatibility breaking risk. I don't think so. The current behaviour is clearly wrong, we dont need a 16x larger stack just because you went from a 4kB to a 64kB base page size. The user application stack usage is the same in both cases. I didn't discuss which behavior is better. Michael said he want to apply his patch to 2.6.32 2.6.33. stable tree never accept the breaking compatibility patch. Your answer doesn't explain why can't we wait it until next merge window. btw, personally, I like page size indepent stack size. but I'm not sure why making stack size independency is related to bug fix. ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] Restrict stack space reservation to rlimit
Hi, I didn't discuss which behavior is better. Michael said he want to apply his patch to 2.6.32 2.6.33. stable tree never accept the breaking compatibility patch. Your answer doesn't explain why can't we wait it until next merge window. btw, personally, I like page size indepent stack size. but I'm not sure why making stack size independency is related to bug fix. OK sorry, I misunderstood your initial mail. I agree fixing the bit that regressed in 2.6.32 is the most important thing. The difference in page size is clearly wrong but since it isn't a regression we could probably live with it until 2.6.34 Anton ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] Restrict stack space reservation to rlimit
Hi apkm, linus: this or something like it needs to go into 2.6.33 ( 32) to fix 'ulimit -s'. fix ulimit -s is too cool explanation ;-) we are not ESPer. please consider to provide what bug is exist. Mikey [PATCH] Restrict stack space reservation to rlimit When reserving stack space for a new process, make sure we're not attempting to allocate more than rlimit allows. Also, reserve the same stack size independent of page size. Why do we need page size independent stack size? It seems to have compatibility breaking risk. This fixes a bug unmasked by fc63cf237078c86214abcb2ee9926d8ad289da9b Signed-off-by: Michael Neuling mi...@neuling.org Cc: Anton Blanchard an...@samba.org Cc: sta...@kernel.org --- fs/exec.c |9 ++--- 1 file changed, 6 insertions(+), 3 deletions(-) Index: clone1/fs/exec.c === --- clone1.orig/fs/exec.c +++ clone1/fs/exec.c @@ -554,7 +554,7 @@ static int shift_arg_pages(struct vm_are return 0; } -#define EXTRA_STACK_VM_PAGES 20 /* random */ +#define EXTRA_STACK_VM_SIZE 81920UL /* randomly 20 4K pages */ /* * Finalizes the stack vm_area_struct. The flags and permissions are updated, @@ -627,10 +627,13 @@ int setup_arg_pages(struct linux_binprm goto out_unlock; } + stack_base = min(EXTRA_STACK_VM_SIZE, + current-signal-rlim[RLIMIT_STACK].rlim_cur) - + PAGE_SIZE; #ifdef CONFIG_STACK_GROWSUP - stack_base = vma-vm_end + EXTRA_STACK_VM_PAGES * PAGE_SIZE; + stack_base = vma-vm_end + stack_base; #else - stack_base = vma-vm_start - EXTRA_STACK_VM_PAGES * PAGE_SIZE; + stack_base = vma-vm_start - stack_base; #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 majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] Restrict stack space reservation to rlimit
Hi, Why do we need page size independent stack size? It seems to have compatibility breaking risk. I don't think so. The current behaviour is clearly wrong, we dont need a 16x larger stack just because you went from a 4kB to a 64kB base page size. The user application stack usage is the same in both cases. I didn't discuss which behavior is better. Michael said he want to apply his patch to 2.6.32 2.6.33. stable tree never accept the breaking compatibility patch. Your answer doesn't explain why can't we wait it until next merge window. btw, personally, I like page size indepent stack size. but I'm not sure why making stack size independency is related to bug fix. I tend to agree. Below is just the bug fix to limit the reservation size based rlimit. We still reserve different stack sizes based on the page size as before (unless we hit rlimit of course). Thanks. I agree your patch in almost part. but I have very few requests. Mikey Restrict stack space reservation to rlimit When reserving stack space for a new process, make sure we're not attempting to allocate more than rlimit allows. This fixes a bug cause by b6a2fea39318e43fee84fa7b0b90d68bed92d2ba mm: variable length argument support and unmasked by fc63cf237078c86214abcb2ee9926d8ad289da9b exec: setup_arg_pages() fails to return errors. Your initial mail have following problem use-case. please append it into the patch description. On recent ppc64 kernels, limiting the stack (using 'ulimit -s blah') is now more restrictive than it was before. On 2.6.31 with 4k pages I could run 'ulimit -s 16; /usr/bin/test' without a problem. Now with mainline, even 'ulimit -s 64; /usr/bin/test' gets killed. Signed-off-by: Michael Neuling mi...@neuling.org Cc: Anton Blanchard an...@samba.org Cc: sta...@kernel.org --- fs/exec.c |7 +-- 1 file changed, 5 insertions(+), 2 deletions(-) Index: linux-2.6-ozlabs/fs/exec.c === --- linux-2.6-ozlabs.orig/fs/exec.c +++ linux-2.6-ozlabs/fs/exec.c @@ -627,10 +627,13 @@ int setup_arg_pages(struct linux_binprm goto out_unlock; } + stack_base = min(EXTRA_STACK_VM_PAGES * PAGE_SIZE, + current-signal-rlim[RLIMIT_STACK].rlim_cur - +PAGE_SIZE); This line is a bit unclear why - PAGE_SIZE is necessary. personally, I like following likes explicit comments. stack_expand = EXTRA_STACK_VM_PAGES * PAGE_SIZE; stack_lim = ACCESS_ONCE(rlim[RLIMIT_STACK].rlim_cur); /* Initial stack must not cause stack overflow. */ if (stack_expand + PAGE_SIZE stack_lim) stack_expand = stack_lim - PAGE_SIZE; note: accessing rlim_cur require ACCESS_ONCE. Thought? #ifdef CONFIG_STACK_GROWSUP - stack_base = vma-vm_end + EXTRA_STACK_VM_PAGES * PAGE_SIZE; + stack_base = vma-vm_end + stack_base; #else - stack_base = vma-vm_start - EXTRA_STACK_VM_PAGES * PAGE_SIZE; + stack_base = vma-vm_start - stack_base; #endif ret = expand_stack(vma, stack_base); if (ret) ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
Re: [PATCH] Restrict stack space reservation to rlimit
Hi, I didn't discuss which behavior is better. Michael said he want to apply his patch to 2.6.32 2.6.33. stable tree never accept the breaking compatibility patch. Your answer doesn't explain why can't we wait it until next merge window. btw, personally, I like page size indepent stack size. but I'm not sure why making stack size independency is related to bug fix. OK sorry, I misunderstood your initial mail. I agree fixing the bit that regressed in 2.6.32 is the most important thing. The difference in page size is clearly wrong but since it isn't a regression we could probably live with it until 2.6.34 thanks! ___ Linuxppc-dev mailing list Linuxppc-dev@lists.ozlabs.org https://lists.ozlabs.org/listinfo/linuxppc-dev
