On Wed 28-06-17 19:52:58, Oleg Nesterov wrote:
> expand_stack(vma) fails if address < stack_guard_gap even if there is no
> vma->vm_prev. I don't think this makes sense, and we didn't do this before
> the recent commit 1be7107fbe18 ("mm: larger stack guard gap, between vmas").
> We do not need a gap in this case, any address is fine as long as
> security_mmap_addr() doesn't object.
>
> This also simplifies the code, we know that address >= prev->vm_end and
> thus underflow is not possible.
>
> Signed-off-by: Oleg Nesterov <o...@redhat.com>
Acked-by: Michal Hocko <mho...@suse.com>
> ---
> mm/mmap.c | 10 +++-------
> 1 file changed, 3 insertions(+), 7 deletions(-)
>
> diff --git a/mm/mmap.c b/mm/mmap.c
> index 8e07976..5a8bd97 100644
> --- a/mm/mmap.c
> +++ b/mm/mmap.c
> @@ -2310,7 +2310,6 @@ int expand_downwards(struct vm_area_struct *vma,
> {
> struct mm_struct *mm = vma->vm_mm;
> struct vm_area_struct *prev;
> - unsigned long gap_addr;
> int error;
>
> address &= PAGE_MASK;
> @@ -2319,14 +2318,11 @@ int expand_downwards(struct vm_area_struct *vma,
> return error;
>
> /* Enforce stack_guard_gap */
> - gap_addr = address - stack_guard_gap;
> - if (gap_addr > address)
> - return -ENOMEM;
I thought this was an underflow protection. address might be still above
min_mmap address while gap_addr can underflow, that would mean that we
might not detect a mapping in that range, but
> prev = vma->vm_prev;
> - if (prev && prev->vm_end > gap_addr) {
> - if (!(prev->vm_flags & VM_GROWSDOWN))
> + /* Check that both stack segments have the same anon_vma? */
> + if (prev && !(prev->vm_flags & VM_GROWSDOWN)) {
> + if (address - prev->vm_end < stack_guard_gap)
this would handle that case properly so the problem wouldn't happen.
> return -ENOMEM;
> - /* Check that both stack segments have the same anon_vma? */
> }
>
> /* We must make sure the anon_vma is allocated. */
> --
> 2.5.0
>
>
--
Michal Hocko
SUSE Labs
