Messages in this thread Patch in this message | | | Date | Mon, 3 Jul 2017 17:49:25 +0200 | From | Michal Hocko <> | Subject | Re: [PATCH 0/1] expand_downwards: don't require the gap if !vm_prev |
| |
On Fri 30-06-17 10:48:15, Linus Torvalds wrote: > On Fri, Jun 30, 2017 at 10:26 AM, Michal Hocko <mhocko@kernel.org> wrote: > > > > Ohh, you misunderstood I guess. They wanted that only for internal > > testing (e.g. make sure that everything that matters blows up if it is > > doing something wrong). Absolutely nothing to base any compilator > > decistion on. > > Oh, good. > > If that's the case, I really think we should try to add some code that > checks that the stack grows strictly one page at a time, and have a > way to enable SIGSEGV if that is ever not the case. > > That should be trivial to add in expand_downwards/expand_upwards.
yes, but I would be worried to have this hardcoded. People very often do run 3rd party code compiled with a non-default distribution compiler. I also expect there are sensible usecases where probing on the stack could lead to a performance degradation so people might want to explicitely disable it.
> We could make a "warn once" thing unconditional for distro testing, > but since compiler people would presumably want to test this before > the rest of the distro is clean, they'd need some rlimit or something > like that to enable it for particular processes.
yes, WARN_ONCE is not very practical to identify offenders..
> Would that be ok for them? > > Some prctl to get/set that "max I'm allowed to extend the stack"?
prctl would be less code than proc interface I've had and quite straightforward as well. Below is what I have ended up with for them. I doesn't warn by default because I thought it would be too noisy without stack probing used more widely.
If you think this is worth pursuing in upstream, just let me know and I can polish it, add a patch for the man page and other things. --- diff --git a/fs/exec.c b/fs/exec.c index 904199086490..7589fb701fca 100644 --- a/fs/exec.c +++ b/fs/exec.c @@ -320,6 +320,13 @@ static int __bprm_mm_init(struct linux_binprm *bprm) arch_bprm_mm_init(mm, vma); up_write(&mm->mmap_sem); bprm->p = vma->vm_end - sizeof(void *); + + /* + * We cannot set the stack expand limit now because we will do manual + * stack expansions which might be larger. See setup_arg_pages + */ + if (current->mm) + bprm->expand_stack_limit = current->mm->expand_stack_limit; return 0; err: up_write(&mm->mmap_sem); @@ -786,6 +793,14 @@ int setup_arg_pages(struct linux_binprm *bprm, if (ret) ret = -EFAULT; + /* + * Stack is finilized now and so all later expansions have + * to comply with the inherited limit. + * + * TODO: Do we want to allow non-privileged task to set + * the limit for privileged ones? + */ + vma->vm_mm->expand_stack_limit = bprm->expand_stack_limit; out_unlock: up_write(&mm->mmap_sem); return ret; diff --git a/include/linux/binfmts.h b/include/linux/binfmts.h index 05488da3aee9..2d3d7fff4811 100644 --- a/include/linux/binfmts.h +++ b/include/linux/binfmts.h @@ -17,6 +17,7 @@ struct linux_binprm { char buf[BINPRM_BUF_SIZE]; #ifdef CONFIG_MMU struct vm_area_struct *vma; + unsigned long expand_stack_limit; unsigned long vma_pages; #else # define MAX_ARG_PAGES 32 diff --git a/include/linux/mm_types.h b/include/linux/mm_types.h index 45cdb27791a3..24ee38a45a9e 100644 --- a/include/linux/mm_types.h +++ b/include/linux/mm_types.h @@ -425,6 +425,7 @@ struct mm_struct { unsigned long start_brk, brk, start_stack; unsigned long arg_start, arg_end, env_start, env_end; + unsigned long expand_stack_limit; /* how much we can grow stack at once */ unsigned long saved_auxv[AT_VECTOR_SIZE]; /* for /proc/PID/auxv */ /* diff --git a/include/uapi/linux/prctl.h b/include/uapi/linux/prctl.h index a8d0759a9e40..6f3c65530c71 100644 --- a/include/uapi/linux/prctl.h +++ b/include/uapi/linux/prctl.h @@ -197,4 +197,7 @@ struct prctl_mm_map { # define PR_CAP_AMBIENT_LOWER 3 # define PR_CAP_AMBIENT_CLEAR_ALL 4 +#define PR_SET_STACK_EXPAND_LIMIT 48 +#define PR_GET_STACK_EXPAND_LIMIT 49 + #endif /* _LINUX_PRCTL_H */ diff --git a/kernel/sys.c b/kernel/sys.c index 4111d584ff4a..05bdcdeda98f 100644 --- a/kernel/sys.c +++ b/kernel/sys.c @@ -2290,6 +2290,17 @@ SYSCALL_DEFINE5(prctl, int, option, unsigned long, arg2, unsigned long, arg3, case PR_GET_FP_MODE: error = GET_FP_MODE(me); break; + case PR_SET_STACK_EXPAND_LIMIT: + if (arg3 || arg4 || arg5) + return -EINVAL; + if (down_write_killable(&me->mm->mmap_sem)) + return -EINTR; + me->mm->expand_stack_limit = arg2; + up_write(&me->mm->mmap_sem); + break; + case PR_GET_STACK_EXPAND_LIMIT: + error = me->mm->expand_stack_limit; + break; default: error = -EINVAL; break; diff --git a/mm/mmap.c b/mm/mmap.c index 5a0ba9788cdd..ff2a5981ee92 100644 --- a/mm/mmap.c +++ b/mm/mmap.c @@ -2266,7 +2266,16 @@ int expand_upwards(struct vm_area_struct *vma, unsigned long address) unsigned long size, grow; size = address - vma->vm_start; - grow = (address - vma->vm_end) >> PAGE_SHIFT; + grow = address - vma->vm_end; + + if (mm->expand_stack_limit && grow > mm->expand_stack_limit) { + pr_warn("%s[%d]: disallowed stack expansion %lu with limit %lu\n", + current->comm, task_pid_nr(current), + grow, mm->expand_stack_limit); + anon_vma_unlock_write(vma->anon_vma); + return -ENOMEM; + } + grow >>= PAGE_SHIFT; error = -ENOMEM; if (vma->vm_pgoff + (size >> PAGE_SHIFT) >= vma->vm_pgoff) { @@ -2350,7 +2359,20 @@ int expand_downwards(struct vm_area_struct *vma, unsigned long size, grow; size = vma->vm_end - address; - grow = (vma->vm_start - address) >> PAGE_SHIFT; + grow = vma->vm_start - address; + + /* + * Make sure that a single stack expansion doesn't exceed the + * configured limit. + */ + if (mm->expand_stack_limit && grow > mm->expand_stack_limit) { + pr_warn("%s[%d]: disallowed stack expansion %lu with limit %lu\n", + current->comm, task_pid_nr(current), + grow, mm->expand_stack_limit); + anon_vma_unlock_write(vma->anon_vma); + return -ENOMEM; + } + grow >>= PAGE_SHIFT; error = -ENOMEM; if (grow <= vma->vm_pgoff) { -- Michal Hocko SUSE Labs
| |