lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite: Windows password security audit tool. GUI, reports in PDF.
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Date:   Sat, 22 Jan 2022 01:53:40 +0000
From:   Liam Howlett <liam.howlett@...cle.com>
To:     Vlastimil Babka <vbabka@...e.cz>
CC:     "maple-tree@...ts.infradead.org" <maple-tree@...ts.infradead.org>,
        "linux-mm@...ck.org" <linux-mm@...ck.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        Andrew Morton <akpm@...ux-foundation.org>,
        Song Liu <songliubraving@...com>,
        Davidlohr Bueso <dave@...olabs.net>,
        "Paul E . McKenney" <paulmck@...nel.org>,
        Matthew Wilcox <willy@...radead.org>,
        Laurent Dufour <ldufour@...ux.ibm.com>,
        David Rientjes <rientjes@...gle.com>,
        Axel Rasmussen <axelrasmussen@...gle.com>,
        Suren Baghdasaryan <surenb@...gle.com>,
        Rik van Riel <riel@...riel.com>,
        Peter Zijlstra <peterz@...radead.org>,
        Michel Lespinasse <walken.cr@...il.com>,
        Jerome Glisse <jglisse@...hat.com>,
        Minchan Kim <minchan@...gle.com>,
        Joel Fernandes <joelaf@...gle.com>,
        Rom Lemarchand <romlem@...gle.com>
Subject: Re: [PATCH v4 27/66] mm/mmap: Change do_brk_munmap() to use
 do_mas_align_munmap()

* Vlastimil Babka <vbabka@...e.cz> [220118 06:57]:
> On 12/1/21 15:30, Liam Howlett wrote:
> > From: "Liam R. Howlett" <Liam.Howlett@...cle.com>
> > 
> > do_brk_munmap() has already aligned the address and has a maple tree
> > state to be used.  Use the new do_mas_align_munmap() to avoid
> > unnecessary alignment and error checks.
> > 
> > Signed-off-by: Liam R. Howlett <Liam.Howlett@...cle.com>
> > ---
> >  mm/mmap.c | 9 ++++++---
> >  1 file changed, 6 insertions(+), 3 deletions(-)
> > 
> > diff --git a/mm/mmap.c b/mm/mmap.c
> > index 14190306a483..79b8494d83c6 100644
> > --- a/mm/mmap.c
> > +++ b/mm/mmap.c
> > @@ -2926,12 +2926,15 @@ static int do_brk_munmap(struct ma_state *mas, struct vm_area_struct *vma,
> >  	struct mm_struct *mm = vma->vm_mm;
> >  	struct vm_area_struct unmap;
> >  	unsigned long unmap_pages;
> > -	int ret = 1;
> > +	int ret;
> >  
> >  	arch_unmap(mm, newbrk, oldbrk);
> >  
> > -	if (likely(vma->vm_start >= newbrk)) { // remove entire mapping(s)
> > -		ret = do_mas_munmap(mas, mm, newbrk, oldbrk-newbrk, uf, true);
> > +	if (likely((vma->vm_end < oldbrk) ||
> > +		   ((vma->vm_start == newbrk) && (vma->vm_end == oldbrk)))) {
> 
> Can you describe this change of conditions in the commit log as well?

Yes, that's fair as it's not easy to see.  The second part is to catch
an exact match of the vma - simple enough.  The first part however, was
changed to catch a tricky mprotect scenario that can land us into a VMA
which is located at a lower address than the end of the brk.  So we know
we have to at least munmap one entire VMA.  Checking the start of the
VMA against the newbrk may not catch this scenario.

And now that I've explained it, I think this change should be backported
to the earlier patch in the series:
mm/mmap: Change do_brk_flags() to expand existing VMA and add do_brk_munmap()

> 
> > +		// remove entire mapping(s)
> > +		ret = do_mas_align_munmap(mas, vma, mm, newbrk, oldbrk, uf,
> > +					  true);
> >  		goto munmap_full_vma;
> >  	}
> >  
> 

Powered by blists - more mailing lists