All of lore.kernel.org
 help / color / mirror / Atom feed
From: Michael Neuling <mikey@neuling.org>
To: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
Cc: stable@kernel.org, aeb@cwi.nl, Oleg Nesterov <oleg@redhat.com>,
	miltonm@bga.com, James Morris <jmorris@namei.org>,
	linuxppc-dev@ozlabs.org, Paul Mackerras <paulus@samba.org>,
	Anton Blanchard <anton@samba.org>,
	Serge Hallyn <serue@us.ibm.com>,
	linux-fsdevel@vger.kernel.org,
	Americo Wang <xiyou.wangcong@gmail.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Ingo Molnar <mingo@elte.hu>,
	linux-kernel@vger.kernel.org,
	Alexander Viro <viro@zeniv.linux.org.uk>
Subject: [PATCH] Restrict initial stack space expansion to rlimit
Date: Tue, 09 Feb 2010 17:11:25 +1100	[thread overview]
Message-ID: <1273.1265695885@neuling.org> (raw)
In-Reply-To: <20100208161014.7C6D.A69D9226@jp.fujitsu.com>

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 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@neuling.org>
Cc: stable@kernel.org
---
Attempts to answer comments from Kosaki Motohiro.

Tested on PPC only, hence !CONFIG_STACK_GROWSUP.  Someone should
probably ACK for an arch with CONFIG_STACK_GROWSUP.

As noted, stable needs the same patch, but 2.6.32 doesn't have the
rlimit() helper.

 fs/exec.c |   21 ++++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)

Index: linux-2.6-ozlabs/fs/exec.c
===================================================================
--- linux-2.6-ozlabs.orig/fs/exec.c
+++ linux-2.6-ozlabs/fs/exec.c
@@ -555,6 +555,7 @@ static int shift_arg_pages(struct vm_are
 }
 
 #define EXTRA_STACK_VM_PAGES	20	/* random */
+#define ALIGN_DOWN(addr,size)	((addr)&(~((size)-1)))
 
 /*
  * Finalizes the stack vm_area_struct. The flags and permissions are updated,
@@ -570,7 +571,7 @@ int setup_arg_pages(struct linux_binprm 
 	struct vm_area_struct *vma = bprm->vma;
 	struct vm_area_struct *prev = NULL;
 	unsigned long vm_flags;
-	unsigned long stack_base;
+	unsigned long stack_base, stack_expand, stack_expand_lim, stack_size;
 
 #ifdef CONFIG_STACK_GROWSUP
 	/* Limit stack size to 1GB */
@@ -627,10 +628,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;
+	if (rlimit(RLIMIT_STACK) < stack_size)
+		stack_expand_lim = 0; /* don't shrick the stack */
+	else
+		/*
+		 * Align this down to a page boundary as expand_stack
+		 * will align it up.
+		 */
+		stack_expand_lim = ALIGN_DOWN(rlimit(RLIMIT_STACK) - stack_size,
+					      PAGE_SIZE);
+	/* Initial stack must not cause stack overflow. */
+	if (stack_expand > stack_expand_lim)
+		stack_expand = stack_expand_lim;
 #ifdef CONFIG_STACK_GROWSUP
-	stack_base = vma->vm_end + EXTRA_STACK_VM_PAGES * PAGE_SIZE;
+	stack_base = vma->vm_end + stack_expand;
 #else
-	stack_base = vma->vm_start - EXTRA_STACK_VM_PAGES * PAGE_SIZE;
+	stack_base = vma->vm_start - stack_expand;
 #endif
 	ret = expand_stack(vma, stack_base);
 	if (ret)

WARNING: multiple messages have this Message-ID (diff)
From: Michael Neuling <mikey@neuling.org>
To: KOSAKI Motohiro <kosaki.motohiro@jp.fujitsu.com>
Cc: Americo Wang <xiyou.wangcong@gmail.com>,
	Anton Blanchard <anton@samba.org>,
	Andrew Morton <akpm@linux-foundation.org>,
	Linus Torvalds <torvalds@linux-foundation.org>,
	Alexander Viro <viro@zeniv.linux.org.uk>,
	Oleg Nesterov <oleg@redhat.com>, James Morris <jmorris@namei.org>,
	Ingo Molnar <mingo@elte.hu>,
	linux-fsdevel@vger.kernel.org, stable@kernel.org,
	linux-kernel@vger.kernel.org, linuxppc-dev@ozlabs.org,
	Serge Hallyn <serue@us.ibm.com>,
	Paul Mackerras <paulus@samba.org>,
	benh@kernel.crashing.org, miltonm@bga.com, aeb@cwi.nl
Subject: [PATCH] Restrict initial stack space expansion to rlimit
Date: Tue, 09 Feb 2010 17:11:25 +1100	[thread overview]
Message-ID: <1273.1265695885@neuling.org> (raw)
In-Reply-To: <20100208161014.7C6D.A69D9226@jp.fujitsu.com>

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 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@neuling.org>
Cc: stable@kernel.org
---
Attempts to answer comments from Kosaki Motohiro.

Tested on PPC only, hence !CONFIG_STACK_GROWSUP.  Someone should
probably ACK for an arch with CONFIG_STACK_GROWSUP.

As noted, stable needs the same patch, but 2.6.32 doesn't have the
rlimit() helper.

 fs/exec.c |   21 ++++++++++++++++++---
 1 file changed, 18 insertions(+), 3 deletions(-)

Index: linux-2.6-ozlabs/fs/exec.c
===================================================================
--- linux-2.6-ozlabs.orig/fs/exec.c
+++ linux-2.6-ozlabs/fs/exec.c
@@ -555,6 +555,7 @@ static int shift_arg_pages(struct vm_are
 }
 
 #define EXTRA_STACK_VM_PAGES	20	/* random */
+#define ALIGN_DOWN(addr,size)	((addr)&(~((size)-1)))
 
 /*
  * Finalizes the stack vm_area_struct. The flags and permissions are updated,
@@ -570,7 +571,7 @@ int setup_arg_pages(struct linux_binprm 
 	struct vm_area_struct *vma = bprm->vma;
 	struct vm_area_struct *prev = NULL;
 	unsigned long vm_flags;
-	unsigned long stack_base;
+	unsigned long stack_base, stack_expand, stack_expand_lim, stack_size;
 
 #ifdef CONFIG_STACK_GROWSUP
 	/* Limit stack size to 1GB */
@@ -627,10 +628,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;
+	if (rlimit(RLIMIT_STACK) < stack_size)
+		stack_expand_lim = 0; /* don't shrick the stack */
+	else
+		/*
+		 * Align this down to a page boundary as expand_stack
+		 * will align it up.
+		 */
+		stack_expand_lim = ALIGN_DOWN(rlimit(RLIMIT_STACK) - stack_size,
+					      PAGE_SIZE);
+	/* Initial stack must not cause stack overflow. */
+	if (stack_expand > stack_expand_lim)
+		stack_expand = stack_expand_lim;
 #ifdef CONFIG_STACK_GROWSUP
-	stack_base = vma->vm_end + EXTRA_STACK_VM_PAGES * PAGE_SIZE;
+	stack_base = vma->vm_end + stack_expand;
 #else
-	stack_base = vma->vm_start - EXTRA_STACK_VM_PAGES * PAGE_SIZE;
+	stack_base = vma->vm_start - stack_expand;
 #endif
 	ret = expand_stack(vma, stack_base);
 	if (ret)

  reply	other threads:[~2010-02-09  6:11 UTC|newest]

Thread overview: 58+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2010-02-06  0:43 Stack size protection broken on ppc64 Michael Neuling
2010-02-06  4:20 ` Anton Blanchard
2010-02-06  4:20   ` Anton Blanchard
2010-02-06 10:22   ` Michael Neuling
2010-02-06 10:22     ` Michael Neuling
2010-02-08  0:04     ` Anton Blanchard
2010-02-08  0:04       ` Anton Blanchard
2010-02-08  0:07     ` [PATCH] Restrict stack space reservation to rlimit Michael Neuling
2010-02-08  0:07       ` Michael Neuling
2010-02-08  0:28       ` Michael Neuling
2010-02-08  0:28         ` Michael Neuling
2010-02-08  5:06       ` KOSAKI Motohiro
2010-02-08  5:06         ` KOSAKI Motohiro
2010-02-08  5:11         ` Anton Blanchard
2010-02-08  5:11           ` Anton Blanchard
2010-02-08  5:22           ` KOSAKI Motohiro
2010-02-08  5:22             ` KOSAKI Motohiro
2010-02-08  5:31             ` Anton Blanchard
2010-02-08  5:31               ` Anton Blanchard
2010-02-08  6:11               ` KOSAKI Motohiro
2010-02-08  6:11                 ` KOSAKI Motohiro
2010-02-08  5:37             ` Michael Neuling
2010-02-08  5:37               ` Michael Neuling
2010-02-08  6:05               ` KOSAKI Motohiro
2010-02-08  6:05                 ` KOSAKI Motohiro
2010-02-08  7:07                 ` Américo Wang
2010-02-08  7:07                   ` Américo Wang
2010-02-08  7:07                   ` Américo Wang
2010-02-08  7:11                   ` KOSAKI Motohiro
2010-02-08  7:11                     ` KOSAKI Motohiro
2010-02-09  6:11                     ` Michael Neuling [this message]
2010-02-09  6:11                       ` [PATCH] Restrict initial stack space expansion " Michael Neuling
2010-02-09  6:46                       ` KOSAKI Motohiro
2010-02-09  6:46                         ` KOSAKI Motohiro
2010-02-09  8:59                         ` Michael Neuling
2010-02-09  8:59                           ` Michael Neuling
2010-02-09 21:25                           ` Andrew Morton
2010-02-09 21:25                             ` Andrew Morton
2010-02-09 21:51                             ` Michael Neuling
2010-02-09 21:51                               ` Michael Neuling
2010-02-09 22:27                               ` Helge Deller
2010-02-09 22:27                                 ` Helge Deller
2010-02-10  5:12                                 ` KOSAKI Motohiro
2010-02-10  5:12                                   ` KOSAKI Motohiro
2010-02-10  5:30                                   ` Michael Neuling
2010-02-10  5:30                                     ` Michael Neuling
2010-02-10  5:31                                   ` Michael Neuling
2010-02-10  5:31                                     ` Michael Neuling
2010-02-11 22:16                                     ` Helge Deller
2010-02-11 22:16                                       ` Helge Deller
2010-02-11 22:22                                       ` Michael Neuling
2010-02-11 22:22                                         ` Michael Neuling
2010-02-12  5:44                             ` [PATCH] Create initial stack independent of PAGE_SIZE Michael Neuling
2010-02-12  7:20                               ` KOSAKI Motohiro
2010-02-12  9:02                                 ` Michael Neuling
2010-02-12  9:51                                   ` KOSAKI Motohiro
2010-02-08 10:45                 ` [PATCH] Restrict stack space reservation to rlimit Michael Neuling
2010-02-08 10:45                   ` Michael Neuling

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=1273.1265695885@neuling.org \
    --to=mikey@neuling.org \
    --cc=aeb@cwi.nl \
    --cc=akpm@linux-foundation.org \
    --cc=anton@samba.org \
    --cc=jmorris@namei.org \
    --cc=kosaki.motohiro@jp.fujitsu.com \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linuxppc-dev@ozlabs.org \
    --cc=miltonm@bga.com \
    --cc=mingo@elte.hu \
    --cc=oleg@redhat.com \
    --cc=paulus@samba.org \
    --cc=serue@us.ibm.com \
    --cc=stable@kernel.org \
    --cc=torvalds@linux-foundation.org \
    --cc=viro@zeniv.linux.org.uk \
    --cc=xiyou.wangcong@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.