oss-sec mailing list archives
[PATCH 2/2] execve: check the VM has enough memory at first
From: KOSAKI Motohiro <kosaki.motohiro () jp fujitsu com>
Date: Thu, 9 Sep 2010 14:04:42 +0900 (JST)
Now, Argv size of execve are limited by STACK_LIMIT/4. In other words, If we are setting 'ulimit -s unlimited', we've lost any guard of argv size. More unfortunately, current argv setup logic is bypassing the VM overcommitment check unintentionally. because current overcommitment check don't care gradually increased stack. therefore, wrong argument of execve() easily makes OOM instead execve() failure. that's bad. After this patch, execve() expand stack at first and receive to check vm_enough_memory() properly. then, too long argument of execve() than the machine memory return EFAULT properly. Note, almost all user are using OVERCOMMIT_GUESS overcommit mode (because it's default). It provide only guess. It doesn't works perfectly on some case. However usually execve() failure is better than OOM-killer and swap-storm compbination. Cc: pageexec () freemail hu Cc: Roland McGrath <roland () redhat com> Cc: Solar Designer <solar () openwall com> Cc: Brad Spengler <spender () grsecurity net> Cc: Eugene Teo <eteo () redhat com> Signed-off-by: KOSAKI Motohiro <kosaki.motohiro () jp fujitsu com> --- fs/compat.c | 38 +++++++++++++++++++++++++++++++------- fs/exec.c | 38 +++++++++++++++++++++++++++++++------- 2 files changed, 62 insertions(+), 14 deletions(-) diff --git a/fs/compat.c b/fs/compat.c index b631120..ff32573 100644 --- a/fs/compat.c +++ b/fs/compat.c @@ -1394,10 +1394,39 @@ static int compat_copy_strings(int argc, compat_uptr_t __user *argv, char *kaddr = NULL; unsigned long kpos = 0; int ret; + compat_uptr_t str; + int len; + int i; + unsigned long total_len = 0; + + for (i = 0; i < argc; i++) { + ret = -EFAULT; + if (get_user(str, argv+i)) + goto out; + len = strnlen_user(compat_ptr(str), MAX_ARG_STRLEN); + if (!len) + goto out; + + ret = -E2BIG; + if (len > MAX_ARG_STRLEN) + goto out; + + total_len += len; + if (total_len > bprm->p) + goto out; + } + + /* + * Firstly, we try to expand stack. It also invoke + * security_vm_enough_memory() and get failure when we don't + * have enough space. It help to avoid stack smashing by plenty argv. + */ + ret = get_user_pages(current, bprm->mm, bprm->p - total_len, + 1, 1, 1, NULL, NULL); + if (ret < 0) + goto out; while (argc-- > 0) { - compat_uptr_t str; - int len; unsigned long pos; if (get_user(str, argv+argc) || @@ -1406,11 +1435,6 @@ static int compat_copy_strings(int argc, compat_uptr_t __user *argv, goto out; } - if (len > MAX_ARG_STRLEN) { - ret = -E2BIG; - goto out; - } - /* We're going to work our way backwords. */ pos = bprm->p; str += len; diff --git a/fs/exec.c b/fs/exec.c index b41834c..ef8b9dc 100644 --- a/fs/exec.c +++ b/fs/exec.c @@ -396,10 +396,39 @@ static int copy_strings(int argc, const char __user *const __user *argv, char *kaddr = NULL; unsigned long kpos = 0; int ret; + int i; + unsigned long total_len = 0; + const char __user *str; + int len; + + for (i = 0; i < argc; i++) { + ret = -EFAULT; + if (get_user(str, argv+i)) + goto out; + len = strnlen_user(str, MAX_ARG_STRLEN); + if (!len) + goto out; + + ret = -E2BIG; + if (!valid_arg_len(bprm, len)) + goto out; + + total_len += len; + if (total_len > bprm->p) + goto out; + } + + /* + * Firstly, we try to expand stack. It also invoke + * security_vm_enough_memory() and get failure when we don't + * have enough space. It help to avoid stack smashing by plenty argv. + */ + ret = get_user_pages(current, bprm->mm, bprm->p - total_len, + 1, 1, 1, NULL, NULL); + if (ret < 0) + goto out; while (argc-- > 0) { - const char __user *str; - int len; unsigned long pos; if (get_user(str, argv+argc) || @@ -408,11 +437,6 @@ static int copy_strings(int argc, const char __user *const __user *argv, goto out; } - if (!valid_arg_len(bprm, len)) { - ret = -E2BIG; - goto out; - } - /* We're going to work our way backwords. */ pos = bprm->p; str += len; -- 1.6.5.2
Current thread:
- Re: [PATCH 1/3] setup_arg_pages: diagnose excessive argument size, (continued)
- Message not available
- Re: [PATCH 1/3] setup_arg_pages: diagnose excessive argument size Roland McGrath (Sep 10)
- [PATCH 2/3] execve: improve interactivity with large arguments Roland McGrath (Sep 07)
- [PATCH 3/3] execve: make responsive to SIGKILL with large arguments Roland McGrath (Sep 07)
- Re: [PATCH 0/3] execve argument-copying fixes KOSAKI Motohiro (Sep 07)
- [PATCH 0/2] execve memory exhaust of argument-copying fixes KOSAKI Motohiro (Sep 09)
- [PATCH 1/2] oom: don't ignore rss in nascent mm KOSAKI Motohiro (Sep 09)
- Message not available
- Re: [PATCH 1/2] oom: don't ignore rss in nascent mm Roland McGrath (Sep 10)
- Message not available
- [PATCH] move cred_guard_mutex from task_struct to signal_struct KOSAKI Motohiro (Sep 10)
- Re: [PATCH] move cred_guard_mutex from task_struct to signal_struct Oleg Nesterov (Sep 10)
- Re: [PATCH] move cred_guard_mutex from task_struct to signal_struct KOSAKI Motohiro (Sep 15)
- [PATCH 2/2] execve: check the VM has enough memory at first KOSAKI Motohiro (Sep 09)
- Re: [PATCH 2/2] execve: check the VM has enough memory at first Linus Torvalds (Sep 10)
- Re: [PATCH 2/2] execve: check the VM has enough memory at first KOSAKI Motohiro (Sep 13)
- Re: [PATCH 2/2] execve: check the VM has enough memory at first KOSAKI Motohiro (Sep 15)
- Re: [PATCH 2/2] execve: check the VM has enough memory at first Linus Torvalds (Sep 16)
- Re: [PATCH] exec argument expansion can inappropriately trigger OOM-killer Solar Designer (Aug 30)
- Re: [PATCH] exec argument expansion can inappropriately trigger OOM-killer Brad Spengler (Aug 30)
- Re: [PATCH] exec argument expansion can inappropriately trigger OOM-killer Solar Designer (Aug 31)
- Re: [PATCH] exec argument expansion can inappropriately triggerOOM-killer Tetsuo Handa (Aug 31)