| From: Kees Cook <keescook@chromium.org> |
| Date: Fri, 23 Jun 2017 15:08:57 -0700 |
| Subject: fs/exec.c: account for argv/envp pointers |
| |
| commit 98da7d08850fb8bdeb395d6368ed15753304aa0c upstream. |
| |
| When limiting the argv/envp strings during exec to 1/4 of the stack limit, |
| the storage of the pointers to the strings was not included. This means |
| that an exec with huge numbers of tiny strings could eat 1/4 of the stack |
| limit in strings and then additional space would be later used by the |
| pointers to the strings. |
| |
| For example, on 32-bit with a 8MB stack rlimit, an exec with 1677721 |
| single-byte strings would consume less than 2MB of stack, the max (8MB / |
| 4) amount allowed, but the pointers to the strings would consume the |
| remaining additional stack space (1677721 * 4 == 6710884). |
| |
| The result (1677721 + 6710884 == 8388605) would exhaust stack space |
| entirely. Controlling this stack exhaustion could result in |
| pathological behavior in setuid binaries (CVE-2017-1000365). |
| |
| [akpm@linux-foundation.org: additional commenting from Kees] |
| Fixes: b6a2fea39318 ("mm: variable length argument support") |
| Link: http://lkml.kernel.org/r/20170622001720.GA32173@beast |
| Signed-off-by: Kees Cook <keescook@chromium.org> |
| Acked-by: Rik van Riel <riel@redhat.com> |
| Acked-by: Michal Hocko <mhocko@suse.com> |
| Cc: Alexander Viro <viro@zeniv.linux.org.uk> |
| Cc: Qualys Security Advisory <qsa@qualys.com> |
| Signed-off-by: Andrew Morton <akpm@linux-foundation.org> |
| Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org> |
| [bwh: Backported to 3.2: use ACCESS_ONCE() instead of READ_ONCE()] |
| Signed-off-by: Ben Hutchings <ben@decadent.org.uk> |
| --- |
| fs/exec.c | 28 ++++++++++++++++++++++++---- |
| 1 file changed, 24 insertions(+), 4 deletions(-) |
| |
| --- a/fs/exec.c |
| +++ b/fs/exec.c |
| @@ -208,8 +208,26 @@ static struct page *get_arg_page(struct |
| |
| if (write) { |
| unsigned long size = bprm->vma->vm_end - bprm->vma->vm_start; |
| + unsigned long ptr_size; |
| struct rlimit *rlim; |
| |
| + /* |
| + * Since the stack will hold pointers to the strings, we |
| + * must account for them as well. |
| + * |
| + * The size calculation is the entire vma while each arg page is |
| + * built, so each time we get here it's calculating how far it |
| + * is currently (rather than each call being just the newly |
| + * added size from the arg page). As a result, we need to |
| + * always add the entire size of the pointers, so that on the |
| + * last call to get_arg_page() we'll actually have the entire |
| + * correct size. |
| + */ |
| + ptr_size = (bprm->argc + bprm->envc) * sizeof(void *); |
| + if (ptr_size > ULONG_MAX - size) |
| + goto fail; |
| + size += ptr_size; |
| + |
| acct_arg_size(bprm, size / PAGE_SIZE); |
| |
| /* |
| @@ -227,13 +245,15 @@ static struct page *get_arg_page(struct |
| * to work from. |
| */ |
| rlim = current->signal->rlim; |
| - if (size > ACCESS_ONCE(rlim[RLIMIT_STACK].rlim_cur) / 4) { |
| - put_page(page); |
| - return NULL; |
| - } |
| + if (size > ACCESS_ONCE(rlim[RLIMIT_STACK].rlim_cur) / 4) |
| + goto fail; |
| } |
| |
| return page; |
| + |
| +fail: |
| + put_page(page); |
| + return NULL; |
| } |
| |
| static void put_arg_page(struct page *page) |