[PATCH] exec: Check stack space more strictly

Path	csiph.com!news.redatomik.org!aioe.org!bofh.it!news.nic.it!robomod
From	Andy Lutomirski <luto@kernel.org>
Newsgroups	linux.kernel
Subject	[PATCH] exec: Check stack space more strictly
Date	Tue, 18 Jul 2017 00:30:02 +0200
Message-ID	<u4mGC-2ui-3@gated-at.bofh.it> (permalink)
X-Original-To	security@kernel.org, Andrew Morton <akpm@linux-foundation.org>
Dmarc-Filter	OpenDMARC Filter v1.3.2 mail.kernel.org 7E46122B67
Authentication-Results	mail.kernel.org; dmarc=none (p=none dis=none) header.from=kernel.org
Authentication-Results	mail.kernel.org; spf=none smtp.mailfrom=luto@kernel.org
X-Mailer	git-send-email 2.9.4
Sender	robomod@news.nic.it
List-ID	<linux-kernel.vger.kernel.org>
X-Mailing-List	linux-kernel@vger.kernel.org
Approved	robomod@news.nic.it
Lines	77
Organization	linux.* mail to news gateway
X-Original-Cc	linux-kernel@vger.kernel.org, Andy Lutomirski <luto@kernel.org>
X-Original-Date	Mon, 17 Jul 2017 15:22:02 -0700
X-Original-Message-ID	<34b776c32a3f75ff155bc85d3c1554e21be7bab2.1500330091.git.luto@kernel.org>
X-Original-Sender	linux-kernel-owner@vger.kernel.org
Xref	csiph.com linux.kernel:1689537

Show key headers only | View raw

We can currently blow past the stack rlimit and cause odd behavior
if there are accounting bugs, rounding issues, or races.  It's not
clear that the odd behavior is actually a problem, but it's nicer to
fail the exec instead of getting out of sync with stack limits.

Improve the code to more carefully check for space and to abort if
our stack mm gets too large in setup_arg_pages().

Signed-off-by: Andy Lutomirski <luto@kernel.org>
---
 fs/exec.c | 44 ++++++++++++++++++++++++++++++++++----------
 1 file changed, 34 insertions(+), 10 deletions(-)

diff --git a/fs/exec.c b/fs/exec.c
index 62175cbcc801..0c60c0495269 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -764,23 +764,47 @@ int setup_arg_pages(struct linux_binprm *bprm,
 	/* mprotect_fixup is overkill to remove the temporary stack flags */
 	vma->vm_flags &= ~VM_STACK_INCOMPLETE_SETUP;
 
-	stack_expand = 131072UL; /* randomly 32*4k (or 2*64k) pages */
-	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;
+	stack_size = vma->vm_end - vma->vm_start;
+
+	if (stack_size > rlim_stack) {
+		/*
+		 * If we've already used too much space (due to accounting
+		 * bugs, alignment, races, or any other cause), bail.
+		 */
+		ret = -ENOMEM;
+		goto out_unlock;
+	}
+
+	/*
+	 * stack_expand is the amount of space beyond the space already used
+	 * that we're going to pre-allocate in our stack.  For historical
+	 * reasons, it's 128kB, unless we have less space than that available
+	 * in our rlimit.
+	 *
+	 * This particular historical wart is wrong-headed, though, since
+	 * we haven't finished binfmt-specific setup, and the binfmt code
+	 * is going to eat up some or all of this space.
+	 */
+	stack_expand = min(rlim_stack - stack_size, 131072UL);
+
 #ifdef CONFIG_STACK_GROWSUP
-	if (stack_size + stack_expand > rlim_stack)
-		stack_base = vma->vm_start + rlim_stack;
-	else
-		stack_base = vma->vm_end + stack_expand;
+	if (TASK_SIZE_MAX - vma->vm_end < stack_expand) {
+		ret = -ENOMEM;
+		goto out_unlock;
+	}
+	stack_base = vma->vm_end + stack_expand;
 #else
-	if (stack_size + stack_expand > rlim_stack)
-		stack_base = vma->vm_end - rlim_stack;
-	else
-		stack_base = vma->vm_start - stack_expand;
+	if (vma->vm_start < mmap_min_addr ||
+	    vma->vm_start - mmap_min_addr < stack_expand) {
+		ret = -ENOMEM;
+		goto out_unlock;
+	}
+	stack_base = vma->vm_start - stack_expand;
 #endif
 	current->mm->start_stack = bprm->p;
 	ret = expand_stack(vma, stack_base);
-- 
2.9.4

Back to linux.kernel | Previous | Next — Next in thread | Find similar | Unroll thread

Thread

[PATCH] exec: Check stack space more strictly Andy Lutomirski <luto@kernel.org> - 2017-07-18 00:30 +0200
  Re: [PATCH] exec: Check stack space more strictly Kees Cook <keescook@chromium.org> - 2017-07-18 01:00 +0200

csiph-web