Re: [PATCH 3.18 32/32] mm: larger stack guard gap, between vmas

[Hugh Dickins]

On Wed, 21 Jun 2017, Willy Tarreau wrote:
> On Tue, Jun 20, 2017 at 10:49:16PM -0700, Hugh Dickins wrote:
> > On Mon, 19 Jun 2017, Greg Kroah-Hartman wrote:
> > 
> > > 3.18-stable review patch.  If anyone has any objections, please let me know.
> > > 
> > > ------------------
> > > 
> > > From: Hugh Dickins <hughd@xxxxxxxxxx>
> > > 
> > > commit 1be7107fbe18eed3e319a6c3e83c78254b693acb upstream.
> > 
> > Here's a few adjustments to the 3.18 patch: no doubt you'll have
> > already sorted out any build errors (and I have to confess that
> > I haven't even tried to build this); and the VM_WARN_ON line (as
> > in 4.4) only fixes a highly unlikely error; but those FOLL_MLOCK
> > lines in mm/gup.c were mistaken, and do need to be deleted.
> 
> Are you sure ? The test on the FOLL_MLOCK flag remains present in
> 4.11 and mainline :
> 
>  	/* mlock all present pages, but do not fault in new pages */
>  	if ((*flags & (FOLL_POPULATE | FOLL_MLOCK)) == FOLL_MLOCK)
>  		return -ENOENT;
> 
> And this test was present although different in 3.18 as well :
> 
> 	/* For mlock, just skip the stack guard page. */
> 	if ((*flags & FOLL_MLOCK) &&
> 			(stack_guard_page_start(vma, address) ||
> 			 stack_guard_page_end(vma, address + PAGE_SIZE)))
> 
> So by removing it we're totally removing any test on FOLL_MLOCK. That
> might be the correct fix, but I'm just a bit surprized since the mainline
> patch doesn't remove it, and only removes the test on FOLL_POPULATE.

I think I'm sure :)  Please take another look, the intention of those
two FOLL_MLOCK tests is completely different.  One of them is about
the mlock2() syscall (I think), which wants not to fault in every
page at syscall time; and the other is about stack guard pages
bogusly included in the vma extents, which must not be faulted in.
The stack guard pages are no longer included in the vma extents,
so we can just delete those lines (note the "&&" in the condition);
but we don't want mlock() to stop faulting its pages in.

Makes sense now, or am I tired and confused?

Hugh