From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1752493AbdF3NQW (ORCPT ); Fri, 30 Jun 2017 09:16:22 -0400 Received: from mx2.suse.de ([195.135.220.15]:33680 "EHLO mx1.suse.de" rhost-flags-OK-OK-OK-FAIL) by vger.kernel.org with ESMTP id S1751911AbdF3NQT (ORCPT ); Fri, 30 Jun 2017 09:16:19 -0400 Date: Fri, 30 Jun 2017 15:16:06 +0200 From: Michal Hocko To: Oleg Nesterov Cc: Hugh Dickins , Andrew Morton , Larry Woodman , Linus Torvalds , linux-kernel@vger.kernel.org Subject: Re: [PATCH 1/1] expand_downwards: don't require the gap if !vm_prev Message-ID: <20170630131605.GJ22917@dhcp22.suse.cz> References: <20170628175237.GA24868@redhat.com> <20170628175258.GA24881@redhat.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <20170628175258.GA24881@redhat.com> User-Agent: Mutt/1.5.23 (2014-03-12) Sender: linux-kernel-owner@vger.kernel.org List-ID: X-Mailing-List: linux-kernel@vger.kernel.org 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 Acked-by: Michal Hocko > --- > 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