Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1671373 > unrolled thread

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

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  Re: [PATCH 3.18 32/32] mm: larger stack guard gap, between vmas Hugh Dickins <hughd@google.com> - 2017-06-21 07:50 +0200
    Re: [PATCH 3.18 32/32] mm: larger stack guard gap, between vmas Willy Tarreau <w@1wt.eu> - 2017-06-21 08:00 +0200
      Re: [PATCH 3.18 32/32] mm: larger stack guard gap, between vmas Hugh Dickins <hughd@google.com> - 2017-06-21 08:20 +0200
        Re: [PATCH 3.18 32/32] mm: larger stack guard gap, between vmas Willy Tarreau <w@1wt.eu> - 2017-06-21 09:30 +0200
    Re: [PATCH 3.18 32/32] mm: larger stack guard gap, between vmas Greg Kroah-Hartman <gregkh@linuxfoundation.org> - 2017-06-24 17:10 +0200

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

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@google.com>
> 
> 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.

Hugh

diff -purN 318n/include/linux/mm.h 318h/include/linux/mm.h
--- 318n/include/linux/mm.h	2017-06-20 16:48:54.050429965 -0700
+++ 318h/include/linux/mm.h	2017-06-20 19:13:05.842191061 -0700
@@ -1242,6 +1242,9 @@ int set_page_dirty_lock(struct page *pag
 int clear_page_dirty_for_io(struct page *page);
 int get_cmdline(struct task_struct *task, char *buffer, int buflen);
 
+extern struct task_struct *task_of_stack(struct task_struct *task,
+				struct vm_area_struct *vma, bool in_group);
+
 extern unsigned long move_page_tables(struct vm_area_struct *vma,
 		unsigned long old_addr, struct vm_area_struct *new_vma,
 		unsigned long new_addr, unsigned long len,
@@ -1897,8 +1900,9 @@ void page_cache_async_readahead(struct a
 				pgoff_t offset,
 				unsigned long size);
 
-extern unsigned long stack_guard_gap;
+unsigned long max_sane_readahead(unsigned long nr);
 
+extern unsigned long stack_guard_gap;
 /* Generic expand stack which grows the stack according to GROWS{UP,DOWN} */
 extern int expand_stack(struct vm_area_struct *vma, unsigned long address);
 
diff -purN 318n/mm/gup.c 318h/mm/gup.c
--- 318n/mm/gup.c	2017-06-20 16:48:54.054429927 -0700
+++ 318h/mm/gup.c	2017-06-20 19:18:19.579275331 -0700
@@ -275,9 +275,6 @@ static int faultin_page(struct task_stru
 	unsigned int fault_flags = 0;
 	int ret;
 
-	/* mlock all present pages, but do not fault in new pages */
-	if (*flags & FOLL_MLOCK)
-		return -ENOENT;
 	if (*flags & FOLL_WRITE)
 		fault_flags |= FAULT_FLAG_WRITE;
 	if (nonblocking)
diff -purN 318n/mm/mmap.c 318h/mm/mmap.c
--- 318n/mm/mmap.c	2017-06-20 16:48:54.054429927 -0700
+++ 318h/mm/mmap.c	2017-06-20 19:43:16.945345744 -0700
@@ -931,7 +931,7 @@ again:			remove_next = 1 + (end > next->
 		else if (next)
 			vma_gap_update(next);
 		else
-			mm->highest_vm_end = end;
+			VM_WARN_ON(mm->highest_vm_end != vm_end_gap(vma));
 	}
 	if (insert && file)
 		uprobe_mmap(insert);

[toc] | [next] | [standalone]

#1671374

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@google.com>
> > 
> > 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.

Thanks,
Willy

[toc] | [prev] | [next] | [standalone]

#1671385

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@google.com>
> > > 
> > > 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

[toc] | [prev] | [next] | [standalone]

#1671435

On Tue, Jun 20, 2017 at 11:10:56PM -0700, Hugh Dickins wrote:
> 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@google.com>
> > > > 
> > > > 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?

Your explanation sounds pretty fine to me, so I agree with you :-)

Thanks!
Willy

[toc] | [prev] | [next] | [standalone]

#1674096

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@google.com>
> > 
> > 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.

Again, many thanks for this, I've now updated my tree with this and will
let it run through 0-day.

greg k-h

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web