* [PATCH] exec: separate MM_ANONPAGES and RLIMIT_STACK accounting
@ 2018-11-12 16:09 Oleg Nesterov
2018-11-13 23:12 ` Kees Cook
2018-11-23 17:17 ` Guenter Roeck
0 siblings, 2 replies; 5+ messages in thread
From: Oleg Nesterov @ 2018-11-12 16:09 UTC (permalink / raw)
To: Andrew Morton
Cc: Ben Woodard, Eric W. Biederman, Kees Cook, Michal Hocko, linux-kernel
get_arg_page() checks bprm->rlim_stack.rlim_cur and re-calculates the
"extra" size for argv/envp pointers every time, this is a bit ugly and
even not strictly correct: acct_arg_size() must not account this size.
Remove all the rlimit code in get_arg_page(). Instead, add bprm->argmin
calculated once at the start of __do_execve_file() and change copy_strings
to check bprm->p >= bprm->argmin.
The patch adds the new helper, prepare_arg_pages() which initializes
bprm->argc/envc and bprm->argmin.
Signed-off-by: Oleg Nesterov <oleg@redhat.com>
---
fs/exec.c | 103 +++++++++++++++++++++++-------------------------
include/linux/binfmts.h | 1 +
2 files changed, 51 insertions(+), 53 deletions(-)
diff --git a/fs/exec.c b/fs/exec.c
index fc281b7..61a5460 100644
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -218,55 +218,10 @@ static struct page *get_arg_page(struct linux_binprm *bprm, unsigned long pos,
if (ret <= 0)
return NULL;
- if (write) {
- unsigned long size = bprm->vma->vm_end - bprm->vma->vm_start;
- unsigned long ptr_size, limit;
-
- /*
- * 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);
-
- /*
- * We've historically supported up to 32 pages (ARG_MAX)
- * of argument strings even with small stacks
- */
- if (size <= ARG_MAX)
- return page;
-
- /*
- * Limit to 1/4 of the max stack size or 3/4 of _STK_LIM
- * (whichever is smaller) for the argv+env strings.
- * This ensures that:
- * - the remaining binfmt code will not run out of stack space,
- * - the program will have a reasonable amount of stack left
- * to work from.
- */
- limit = _STK_LIM / 4 * 3;
- limit = min(limit, bprm->rlim_stack.rlim_cur / 4);
- if (size > limit)
- goto fail;
- }
+ if (write)
+ acct_arg_size(bprm, vma_pages(bprm->vma));
return page;
-
-fail:
- put_page(page);
- return NULL;
}
static void put_arg_page(struct page *page)
@@ -492,6 +447,50 @@ static int count(struct user_arg_ptr argv, int max)
return i;
}
+static int prepare_arg_pages(struct linux_binprm *bprm,
+ struct user_arg_ptr argv, struct user_arg_ptr envp)
+{
+ unsigned long limit, ptr_size;
+
+ bprm->argc = count(argv, MAX_ARG_STRINGS);
+ if (bprm->argc < 0)
+ return bprm->argc;
+
+ bprm->envc = count(envp, MAX_ARG_STRINGS);
+ if (bprm->envc < 0)
+ return bprm->envc;
+
+ /*
+ * Limit to 1/4 of the max stack size or 3/4 of _STK_LIM
+ * (whichever is smaller) for the argv+env strings.
+ * This ensures that:
+ * - the remaining binfmt code will not run out of stack space,
+ * - the program will have a reasonable amount of stack left
+ * to work from.
+ */
+ limit = _STK_LIM / 4 * 3;
+ limit = min(limit, bprm->rlim_stack.rlim_cur / 4);
+ /*
+ * We've historically supported up to 32 pages (ARG_MAX)
+ * of argument strings even with small stacks
+ */
+ limit = max(limit, (unsigned long)ARG_MAX);
+ /*
+ * We must account for the size of all the argv and envp pointers to
+ * the argv and envp strings, since they will also take up space in
+ * the stack. They aren't stored until much later when we can't
+ * signal to the parent that the child has run out of stack space.
+ * Instead, calculate it here so it's possible to fail gracefully.
+ */
+ ptr_size = (bprm->argc + bprm->envc) * sizeof(void *);
+ if (limit <= ptr_size)
+ return -E2BIG;
+ limit -= ptr_size;
+
+ bprm->argmin = bprm->p - limit;
+ return 0;
+}
+
/*
* 'copy_strings()' copies argument/environment strings from the old
* processes's memory to the new process's stack. The call to get_user_pages()
@@ -527,6 +526,8 @@ static int copy_strings(int argc, struct user_arg_ptr argv,
pos = bprm->p;
str += len;
bprm->p -= len;
+ if (bprm->p < bprm->argmin)
+ goto out;
while (len > 0) {
int offset, bytes_to_copy;
@@ -1789,12 +1790,8 @@ static int __do_execve_file(int fd, struct filename *filename,
if (retval)
goto out_unmark;
- bprm->argc = count(argv, MAX_ARG_STRINGS);
- if ((retval = bprm->argc) < 0)
- goto out;
-
- bprm->envc = count(envp, MAX_ARG_STRINGS);
- if ((retval = bprm->envc) < 0)
+ retval = prepare_arg_pages(bprm, argv, envp);
+ if (retval < 0)
goto out;
retval = prepare_binprm(bprm);
diff --git a/include/linux/binfmts.h b/include/linux/binfmts.h
index e9f5fe6..03200a8 100644
--- a/include/linux/binfmts.h
+++ b/include/linux/binfmts.h
@@ -25,6 +25,7 @@ struct linux_binprm {
#endif
struct mm_struct *mm;
unsigned long p; /* current top of mem */
+ unsigned long argmin; /* rlimit marker for copy_strings() */
unsigned int
/*
* True after the bprm_set_creds hook has been called once
--
2.5.0
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] exec: separate MM_ANONPAGES and RLIMIT_STACK accounting
2018-11-12 16:09 [PATCH] exec: separate MM_ANONPAGES and RLIMIT_STACK accounting Oleg Nesterov
@ 2018-11-13 23:12 ` Kees Cook
2018-11-23 17:17 ` Guenter Roeck
1 sibling, 0 replies; 5+ messages in thread
From: Kees Cook @ 2018-11-13 23:12 UTC (permalink / raw)
To: Oleg Nesterov
Cc: Andrew Morton, Ben Woodard, Eric W. Biederman, Michal Hocko, LKML
On Mon, Nov 12, 2018 at 10:09 AM, Oleg Nesterov <oleg@redhat.com> wrote:
> get_arg_page() checks bprm->rlim_stack.rlim_cur and re-calculates the
> "extra" size for argv/envp pointers every time, this is a bit ugly and
> even not strictly correct: acct_arg_size() must not account this size.
>
> Remove all the rlimit code in get_arg_page(). Instead, add bprm->argmin
> calculated once at the start of __do_execve_file() and change copy_strings
> to check bprm->p >= bprm->argmin.
>
> The patch adds the new helper, prepare_arg_pages() which initializes
> bprm->argc/envc and bprm->argmin.
>
> Signed-off-by: Oleg Nesterov <oleg@redhat.com>
Acked-by: Kees Cook <keescook@chromium.org>
Thanks for nailing this all down. :)
-Kees
> ---
> fs/exec.c | 103 +++++++++++++++++++++++-------------------------
> include/linux/binfmts.h | 1 +
> 2 files changed, 51 insertions(+), 53 deletions(-)
>
> diff --git a/fs/exec.c b/fs/exec.c
> index fc281b7..61a5460 100644
> --- a/fs/exec.c
> +++ b/fs/exec.c
> @@ -218,55 +218,10 @@ static struct page *get_arg_page(struct linux_binprm *bprm, unsigned long pos,
> if (ret <= 0)
> return NULL;
>
> - if (write) {
> - unsigned long size = bprm->vma->vm_end - bprm->vma->vm_start;
> - unsigned long ptr_size, limit;
> -
> - /*
> - * 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);
> -
> - /*
> - * We've historically supported up to 32 pages (ARG_MAX)
> - * of argument strings even with small stacks
> - */
> - if (size <= ARG_MAX)
> - return page;
> -
> - /*
> - * Limit to 1/4 of the max stack size or 3/4 of _STK_LIM
> - * (whichever is smaller) for the argv+env strings.
> - * This ensures that:
> - * - the remaining binfmt code will not run out of stack space,
> - * - the program will have a reasonable amount of stack left
> - * to work from.
> - */
> - limit = _STK_LIM / 4 * 3;
> - limit = min(limit, bprm->rlim_stack.rlim_cur / 4);
> - if (size > limit)
> - goto fail;
> - }
> + if (write)
> + acct_arg_size(bprm, vma_pages(bprm->vma));
>
> return page;
> -
> -fail:
> - put_page(page);
> - return NULL;
> }
>
> static void put_arg_page(struct page *page)
> @@ -492,6 +447,50 @@ static int count(struct user_arg_ptr argv, int max)
> return i;
> }
>
> +static int prepare_arg_pages(struct linux_binprm *bprm,
> + struct user_arg_ptr argv, struct user_arg_ptr envp)
> +{
> + unsigned long limit, ptr_size;
> +
> + bprm->argc = count(argv, MAX_ARG_STRINGS);
> + if (bprm->argc < 0)
> + return bprm->argc;
> +
> + bprm->envc = count(envp, MAX_ARG_STRINGS);
> + if (bprm->envc < 0)
> + return bprm->envc;
> +
> + /*
> + * Limit to 1/4 of the max stack size or 3/4 of _STK_LIM
> + * (whichever is smaller) for the argv+env strings.
> + * This ensures that:
> + * - the remaining binfmt code will not run out of stack space,
> + * - the program will have a reasonable amount of stack left
> + * to work from.
> + */
> + limit = _STK_LIM / 4 * 3;
> + limit = min(limit, bprm->rlim_stack.rlim_cur / 4);
> + /*
> + * We've historically supported up to 32 pages (ARG_MAX)
> + * of argument strings even with small stacks
> + */
> + limit = max(limit, (unsigned long)ARG_MAX);
> + /*
> + * We must account for the size of all the argv and envp pointers to
> + * the argv and envp strings, since they will also take up space in
> + * the stack. They aren't stored until much later when we can't
> + * signal to the parent that the child has run out of stack space.
> + * Instead, calculate it here so it's possible to fail gracefully.
> + */
> + ptr_size = (bprm->argc + bprm->envc) * sizeof(void *);
> + if (limit <= ptr_size)
> + return -E2BIG;
> + limit -= ptr_size;
> +
> + bprm->argmin = bprm->p - limit;
> + return 0;
> +}
> +
> /*
> * 'copy_strings()' copies argument/environment strings from the old
> * processes's memory to the new process's stack. The call to get_user_pages()
> @@ -527,6 +526,8 @@ static int copy_strings(int argc, struct user_arg_ptr argv,
> pos = bprm->p;
> str += len;
> bprm->p -= len;
> + if (bprm->p < bprm->argmin)
> + goto out;
>
> while (len > 0) {
> int offset, bytes_to_copy;
> @@ -1789,12 +1790,8 @@ static int __do_execve_file(int fd, struct filename *filename,
> if (retval)
> goto out_unmark;
>
> - bprm->argc = count(argv, MAX_ARG_STRINGS);
> - if ((retval = bprm->argc) < 0)
> - goto out;
> -
> - bprm->envc = count(envp, MAX_ARG_STRINGS);
> - if ((retval = bprm->envc) < 0)
> + retval = prepare_arg_pages(bprm, argv, envp);
> + if (retval < 0)
> goto out;
>
> retval = prepare_binprm(bprm);
> diff --git a/include/linux/binfmts.h b/include/linux/binfmts.h
> index e9f5fe6..03200a8 100644
> --- a/include/linux/binfmts.h
> +++ b/include/linux/binfmts.h
> @@ -25,6 +25,7 @@ struct linux_binprm {
> #endif
> struct mm_struct *mm;
> unsigned long p; /* current top of mem */
> + unsigned long argmin; /* rlimit marker for copy_strings() */
> unsigned int
> /*
> * True after the bprm_set_creds hook has been called once
> --
> 2.5.0
>
>
--
Kees Cook
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] exec: separate MM_ANONPAGES and RLIMIT_STACK accounting
2018-11-12 16:09 [PATCH] exec: separate MM_ANONPAGES and RLIMIT_STACK accounting Oleg Nesterov
2018-11-13 23:12 ` Kees Cook
@ 2018-11-23 17:17 ` Guenter Roeck
2018-11-26 12:23 ` Oleg Nesterov
1 sibling, 1 reply; 5+ messages in thread
From: Guenter Roeck @ 2018-11-23 17:17 UTC (permalink / raw)
To: Oleg Nesterov
Cc: Andrew Morton, Ben Woodard, Eric W. Biederman, Kees Cook,
Michal Hocko, linux-kernel
Hi,
On Mon, Nov 12, 2018 at 05:09:10PM +0100, Oleg Nesterov wrote:
> get_arg_page() checks bprm->rlim_stack.rlim_cur and re-calculates the
> "extra" size for argv/envp pointers every time, this is a bit ugly and
> even not strictly correct: acct_arg_size() must not account this size.
>
> Remove all the rlimit code in get_arg_page(). Instead, add bprm->argmin
> calculated once at the start of __do_execve_file() and change copy_strings
> to check bprm->p >= bprm->argmin.
>
> The patch adds the new helper, prepare_arg_pages() which initializes
> bprm->argc/envc and bprm->argmin.
>
> Signed-off-by: Oleg Nesterov <oleg@redhat.com>
> Acked-by: Kees Cook <keescook@chromium.org>
This patch results in various qemu boot failures in -next. Bisect logs
are attached. It looks like all nommu boots are failing. I bisected an385
and m68k, but xtensa (kc705-nommu) fails with the same error.
[ 5.416926] Run /sbin/init as init process
[ 5.419792] Failed to execute /sbin/init (error -7)
[ 5.421128] Run /sbin/init as init process
[ 5.423284] Starting init: /sbin/init exists but couldn't execute it (error -7)
[ 5.424747] Run /etc/init as init process
[ 5.428507] Run /bin/init as init process
[ 5.430478] Run /bin/sh as init process
[ 5.433179] Starting init: /bin/sh exists but couldn't execute it (error -7)
Guenter
---
an385:
# bad: [8c9733fd9806c71e7f2313a280f98cb3051f93df] Add linux-next specific files for 20181123
# good: [9ff01193a20d391e8dbce4403dd5ef87c7eaaca6] Linux 4.20-rc3
git bisect start 'HEAD' 'v4.20-rc3'
# good: [34c2101b4f765edf1b91c2837da9c60fbf9f6912] Merge remote-tracking branch 'spi-nor/spi-nor/next'
git bisect good 34c2101b4f765edf1b91c2837da9c60fbf9f6912
# good: [15367a0657fc8027ff3466bf0202bde9f270259b] Merge remote-tracking branch 'kgdb/kgdb-next'
git bisect good 15367a0657fc8027ff3466bf0202bde9f270259b
# good: [d29686ab179c34c5dbaac067a9effbeeb6a8073e] Merge remote-tracking branch 'soundwire/next'
git bisect good d29686ab179c34c5dbaac067a9effbeeb6a8073e
# good: [3381311f9a261c9d2863f0d52e9499c88ccb1f44] Merge remote-tracking branch 'cgroup/for-next'
git bisect good 3381311f9a261c9d2863f0d52e9499c88ccb1f44
# good: [b5bf9c4c28a25b74475c3f3d3fe6d5d737629ab7] Merge remote-tracking branch 'pinctrl/for-next'
git bisect good b5bf9c4c28a25b74475c3f3d3fe6d5d737629ab7
# good: [a75f6952444a401f5464e977caff2a84118b66c9] fs/epoll: deal with wait_queue only once
git bisect good a75f6952444a401f5464e977caff2a84118b66c9
# good: [d83de16290eefb4372520a61744b11bddb6e8f7e] Merge remote-tracking branch 'slimbus/for-next'
git bisect good d83de16290eefb4372520a61744b11bddb6e8f7e
# good: [f2d2d58687c8300fe4ebf09d78b26cd68c825df7] Merge remote-tracking branch 'xarray/xarray'
git bisect good f2d2d58687c8300fe4ebf09d78b26cd68c825df7
# bad: [eac22eb65c3cc96d8cb2c7aac7dd816030a86955] ipc: allow boot time extension of IPCMNI from 32k to 8M
git bisect bad eac22eb65c3cc96d8cb2c7aac7dd816030a86955
# good: [fb95dde314b136e9598a56350b3de3994d272486] exec: increase BINPRM_BUF_SIZE to 256
git bisect good fb95dde314b136e9598a56350b3de3994d272486
# bad: [fb704fe633a3ca80a68282d5c3c665c4382f500b] exec-separate-mm_anonpages-and-rlimit_stack-accounting-checkpatch-fixes
git bisect bad fb704fe633a3ca80a68282d5c3c665c4382f500b
# bad: [eed684faf14610c2063c1f03e0a1f5ef56f23bb4] exec: separate MM_ANONPAGES and RLIMIT_STACK accounting
git bisect bad eed684faf14610c2063c1f03e0a1f5ef56f23bb4
# first bad commit: [eed684faf14610c2063c1f03e0a1f5ef56f23bb4] exec: separate MM_ANONPAGES and RLIMIT_STACK accounting
---
m68k:
# bad: [8c9733fd9806c71e7f2313a280f98cb3051f93df] Add linux-next specific files for 20181123
# good: [9ff01193a20d391e8dbce4403dd5ef87c7eaaca6] Linux 4.20-rc3
git bisect start 'HEAD' 'v4.20-rc3'
# good: [34c2101b4f765edf1b91c2837da9c60fbf9f6912] Merge remote-tracking branch 'spi-nor/spi-nor/next'
git bisect good 34c2101b4f765edf1b91c2837da9c60fbf9f6912
# good: [15367a0657fc8027ff3466bf0202bde9f270259b] Merge remote-tracking branch 'kgdb/kgdb-next'
git bisect good 15367a0657fc8027ff3466bf0202bde9f270259b
# good: [d29686ab179c34c5dbaac067a9effbeeb6a8073e] Merge remote-tracking branch 'soundwire/next'
git bisect good d29686ab179c34c5dbaac067a9effbeeb6a8073e
# good: [3381311f9a261c9d2863f0d52e9499c88ccb1f44] Merge remote-tracking branch 'cgroup/for-next'
git bisect good 3381311f9a261c9d2863f0d52e9499c88ccb1f44
# good: [b5bf9c4c28a25b74475c3f3d3fe6d5d737629ab7] Merge remote-tracking branch 'pinctrl/for-next'
git bisect good b5bf9c4c28a25b74475c3f3d3fe6d5d737629ab7
# good: [a75f6952444a401f5464e977caff2a84118b66c9] fs/epoll: deal with wait_queue only once
git bisect good a75f6952444a401f5464e977caff2a84118b66c9
# good: [d83de16290eefb4372520a61744b11bddb6e8f7e] Merge remote-tracking branch 'slimbus/for-next'
git bisect good d83de16290eefb4372520a61744b11bddb6e8f7e
# good: [f2d2d58687c8300fe4ebf09d78b26cd68c825df7] Merge remote-tracking branch 'xarray/xarray'
git bisect good f2d2d58687c8300fe4ebf09d78b26cd68c825df7
# bad: [eac22eb65c3cc96d8cb2c7aac7dd816030a86955] ipc: allow boot time extension of IPCMNI from 32k to 8M
git bisect bad eac22eb65c3cc96d8cb2c7aac7dd816030a86955
# good: [fb95dde314b136e9598a56350b3de3994d272486] exec: increase BINPRM_BUF_SIZE to 256
git bisect good fb95dde314b136e9598a56350b3de3994d272486
# bad: [fb704fe633a3ca80a68282d5c3c665c4382f500b] exec-separate-mm_anonpages-and-rlimit_stack-accounting-checkpatch-fixes
git bisect bad fb704fe633a3ca80a68282d5c3c665c4382f500b
# bad: [eed684faf14610c2063c1f03e0a1f5ef56f23bb4] exec: separate MM_ANONPAGES and RLIMIT_STACK accounting
git bisect bad eed684faf14610c2063c1f03e0a1f5ef56f23bb4
# first bad commit: [eed684faf14610c2063c1f03e0a1f5ef56f23bb4] exec: separate MM_ANONPAGES and RLIMIT_STACK accounting
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] exec: separate MM_ANONPAGES and RLIMIT_STACK accounting
2018-11-23 17:17 ` Guenter Roeck
@ 2018-11-26 12:23 ` Oleg Nesterov
2018-11-26 17:34 ` Guenter Roeck
0 siblings, 1 reply; 5+ messages in thread
From: Oleg Nesterov @ 2018-11-26 12:23 UTC (permalink / raw)
To: Guenter Roeck
Cc: Andrew Morton, Ben Woodard, Eric W. Biederman, Kees Cook,
Michal Hocko, linux-kernel
Hi Guenter,
On 11/23, Guenter Roeck wrote:
>
> On Mon, Nov 12, 2018 at 05:09:10PM +0100, Oleg Nesterov wrote:
> > get_arg_page() checks bprm->rlim_stack.rlim_cur and re-calculates the
> > "extra" size for argv/envp pointers every time, this is a bit ugly and
> > even not strictly correct: acct_arg_size() must not account this size.
> >
> > Remove all the rlimit code in get_arg_page(). Instead, add bprm->argmin
> > calculated once at the start of __do_execve_file() and change copy_strings
> > to check bprm->p >= bprm->argmin.
> >
> > The patch adds the new helper, prepare_arg_pages() which initializes
> > bprm->argc/envc and bprm->argmin.
> >
> > Signed-off-by: Oleg Nesterov <oleg@redhat.com>
> > Acked-by: Kees Cook <keescook@chromium.org>
>
> This patch results in various qemu boot failures in -next. Bisect logs
> are attached. It looks like all nommu boots are failing.
^^^^^
Ah, thanks.
Yes, I forgot about the !CONFIG_MMU version of get_arg_page() which doesn't
check RLIMIT_STACK at all.
I'll send the trivial fix. Meanwile, could you test the patch below? to ensure
this is the only problem.
Oleg.
--- a/fs/exec.c
+++ b/fs/exec.c
@@ -526,8 +526,10 @@ static int copy_strings(int argc, struct user_arg_ptr argv,
pos = bprm->p;
str += len;
bprm->p -= len;
+#ifdef CONFIG_MMU
if (bprm->p < bprm->argmin)
goto out;
+#endif
while (len > 0) {
int offset, bytes_to_copy;
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [PATCH] exec: separate MM_ANONPAGES and RLIMIT_STACK accounting
2018-11-26 12:23 ` Oleg Nesterov
@ 2018-11-26 17:34 ` Guenter Roeck
0 siblings, 0 replies; 5+ messages in thread
From: Guenter Roeck @ 2018-11-26 17:34 UTC (permalink / raw)
To: Oleg Nesterov
Cc: Andrew Morton, Ben Woodard, Eric W. Biederman, Kees Cook,
Michal Hocko, linux-kernel
Hi Oleg,
On Mon, Nov 26, 2018 at 01:23:07PM +0100, Oleg Nesterov wrote:
> Hi Guenter,
>
> On 11/23, Guenter Roeck wrote:
> >
> > On Mon, Nov 12, 2018 at 05:09:10PM +0100, Oleg Nesterov wrote:
> > > get_arg_page() checks bprm->rlim_stack.rlim_cur and re-calculates the
> > > "extra" size for argv/envp pointers every time, this is a bit ugly and
> > > even not strictly correct: acct_arg_size() must not account this size.
> > >
> > > Remove all the rlimit code in get_arg_page(). Instead, add bprm->argmin
> > > calculated once at the start of __do_execve_file() and change copy_strings
> > > to check bprm->p >= bprm->argmin.
> > >
> > > The patch adds the new helper, prepare_arg_pages() which initializes
> > > bprm->argc/envc and bprm->argmin.
> > >
> > > Signed-off-by: Oleg Nesterov <oleg@redhat.com>
> > > Acked-by: Kees Cook <keescook@chromium.org>
> >
> > This patch results in various qemu boot failures in -next. Bisect logs
> > are attached. It looks like all nommu boots are failing.
> ^^^^^
>
> Ah, thanks.
>
> Yes, I forgot about the !CONFIG_MMU version of get_arg_page() which doesn't
> check RLIMIT_STACK at all.
>
> I'll send the trivial fix. Meanwile, could you test the patch below? to ensure
> this is the only problem.
>
Tested-by: Guenter Roeck <linux@roeck-us.net>
with arm:mps2-an385, xtensa:kc705-nommu, and m68k:mcf5208evb.
Thanks,
Guenter
> Oleg.
>
> --- a/fs/exec.c
> +++ b/fs/exec.c
> @@ -526,8 +526,10 @@ static int copy_strings(int argc, struct user_arg_ptr argv,
> pos = bprm->p;
> str += len;
> bprm->p -= len;
> +#ifdef CONFIG_MMU
> if (bprm->p < bprm->argmin)
> goto out;
> +#endif
>
> while (len > 0) {
> int offset, bytes_to_copy;
>
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2018-11-26 17:34 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2018-11-12 16:09 [PATCH] exec: separate MM_ANONPAGES and RLIMIT_STACK accounting Oleg Nesterov
2018-11-13 23:12 ` Kees Cook
2018-11-23 17:17 ` Guenter Roeck
2018-11-26 12:23 ` Oleg Nesterov
2018-11-26 17:34 ` Guenter Roeck
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).