Re: [PATCH] mm: mm, mmap: do not blow on PROT_NONE MAP_FIXED holes in the stack

[Michal Hocko]

On Wed 05-07-17 14:18:49, Andrew Morton wrote:
> On Wed, 5 Jul 2017 20:28:49 +0200 Michal Hocko  wrote:
> 
> > "mm: enlarge stack guard gap" has introduced a regression in some rust
> > and Java environments which are trying to implement their own stack
> > guard page.  They are punching a new MAP_FIXED mapping inside the
> > existing stack Vma.
> > 
> > This will confuse expand_{downwards,upwards} into thinking that the stack
> > expansion would in fact get us too close to an existing non-stack vma
> > which is a correct behavior wrt. safety. It is a real regression on
> > the other hand. Let's work around the problem by considering PROT_NONE
> > mapping as a part of the stack. This is a gros hack but overflowing to
> > such a mapping would trap anyway an we only can hope that usespace
> > knows what it is doing and handle it propely.
> > 
> > Fixes: d4d2d35e6ef9 ("mm: larger stack guard gap, between vmas")
> 
> That should be 1be7107fbe18, yes?

yes. d4d2d35e6ef9 was a cherry-pick into the mmotm git tree. Sorry about
that.

> > Debugged-by: Vlastimil Babka 
> > Signed-off-by: Michal Hocko 

-- 
Michal Hocko
SUSE Labs

Re: [PATCH] mm: mm, mmap: do not blow on PROT_NONE MAP_FIXED holes in the stack

[Michal Hocko]

On Wed 05-07-17 14:18:49, Andrew Morton wrote:
> On Wed, 5 Jul 2017 20:28:49 +0200 Michal Hocko  wrote:
> 
> > "mm: enlarge stack guard gap" has introduced a regression in some rust
> > and Java environments which are trying to implement their own stack
> > guard page.  They are punching a new MAP_FIXED mapping inside the
> > existing stack Vma.
> > 
> > This will confuse expand_{downwards,upwards} into thinking that the stack
> > expansion would in fact get us too close to an existing non-stack vma
> > which is a correct behavior wrt. safety. It is a real regression on
> > the other hand. Let's work around the problem by considering PROT_NONE
> > mapping as a part of the stack. This is a gros hack but overflowing to
> > such a mapping would trap anyway an we only can hope that usespace
> > knows what it is doing and handle it propely.
> > 
> > Fixes: d4d2d35e6ef9 ("mm: larger stack guard gap, between vmas")
> 
> That should be 1be7107fbe18, yes?

yes. d4d2d35e6ef9 was a cherry-pick into the mmotm git tree. Sorry about
that.

> > Debugged-by: Vlastimil Babka 
> > Signed-off-by: Michal Hocko 

-- 
Michal Hocko
SUSE Labs

Re: [PATCH] mm: mm, mmap: do not blow on PROT_NONE MAP_FIXED holes in the stack

[Linus Torvalds]

On Wed, Jul 5, 2017 at 2:18 PM, Andrew Morton  wrote:
> On Wed, 5 Jul 2017 20:28:49 +0200 Michal Hocko  wrote:
>>>
>> Fixes: d4d2d35e6ef9 ("mm: larger stack guard gap, between vmas")
>
> That should be 1be7107fbe18, yes?

Good catch. I assume the d4d2d35e6ef9 is one of the stable backport commits..

  Linus

Re: [PATCH] mm: mm, mmap: do not blow on PROT_NONE MAP_FIXED holes in the stack

[Linus Torvalds]

On Wed, Jul 5, 2017 at 2:18 PM, Andrew Morton  wrote:
> On Wed, 5 Jul 2017 20:28:49 +0200 Michal Hocko  wrote:
>>>
>> Fixes: d4d2d35e6ef9 ("mm: larger stack guard gap, between vmas")
>
> That should be 1be7107fbe18, yes?

Good catch. I assume the d4d2d35e6ef9 is one of the stable backport commits..

  Linus

Re: [PATCH] mm: mm, mmap: do not blow on PROT_NONE MAP_FIXED holes in the stack

[Andrew Morton]

On Wed, 5 Jul 2017 20:28:49 +0200 Michal Hocko  wrote:

> "mm: enlarge stack guard gap" has introduced a regression in some rust
> and Java environments which are trying to implement their own stack
> guard page.  They are punching a new MAP_FIXED mapping inside the
> existing stack Vma.
> 
> This will confuse expand_{downwards,upwards} into thinking that the stack
> expansion would in fact get us too close to an existing non-stack vma
> which is a correct behavior wrt. safety. It is a real regression on
> the other hand. Let's work around the problem by considering PROT_NONE
> mapping as a part of the stack. This is a gros hack but overflowing to
> such a mapping would trap anyway an we only can hope that usespace
> knows what it is doing and handle it propely.
> 
> Fixes: d4d2d35e6ef9 ("mm: larger stack guard gap, between vmas")

That should be 1be7107fbe18, yes?

> Debugged-by: Vlastimil Babka 
> Signed-off-by: Michal Hocko 

Re: [PATCH] mm: mm, mmap: do not blow on PROT_NONE MAP_FIXED holes in the stack

[Andrew Morton]

On Wed, 5 Jul 2017 20:28:49 +0200 Michal Hocko  wrote:

> "mm: enlarge stack guard gap" has introduced a regression in some rust
> and Java environments which are trying to implement their own stack
> guard page.  They are punching a new MAP_FIXED mapping inside the
> existing stack Vma.
> 
> This will confuse expand_{downwards,upwards} into thinking that the stack
> expansion would in fact get us too close to an existing non-stack vma
> which is a correct behavior wrt. safety. It is a real regression on
> the other hand. Let's work around the problem by considering PROT_NONE
> mapping as a part of the stack. This is a gros hack but overflowing to
> such a mapping would trap anyway an we only can hope that usespace
> knows what it is doing and handle it propely.
> 
> Fixes: d4d2d35e6ef9 ("mm: larger stack guard gap, between vmas")

That should be 1be7107fbe18, yes?

> Debugged-by: Vlastimil Babka 
> Signed-off-by: Michal Hocko 

Re: [PATCH] mm: mm, mmap: do not blow on PROT_NONE MAP_FIXED holes in the stack

[Willy Tarreau]

On Wed, Jul 05, 2017 at 12:15:05PM -0700, Linus Torvalds wrote:
> On Wed, Jul 5, 2017 at 11:53 AM, Michal Hocko  wrote:
> >
> > That would lead to conflicts when backporting to stable trees though
> > which is quite annoying as well and arguably slightly more annoying than
> > resolving this in mmotm. I can help to rebase Oleg's patch on top of
> > mine which is not a stable material.
> 
> Ok, fair enough - I was actually expecting that Oleg's patch would
> just be marked for stable too just to keep differences minimal.
> 
> But yes, putting your patch in first and then Oleg's on top means that
> it works regardless.
> 
> Any opinions from others?

No pb here, and this one is reasonably easy to backport anyway as the
test is easy to locate.

Willy

Re: [PATCH] mm: mm, mmap: do not blow on PROT_NONE MAP_FIXED holes in the stack

[Willy Tarreau]

On Wed, Jul 05, 2017 at 12:15:05PM -0700, Linus Torvalds wrote:
> On Wed, Jul 5, 2017 at 11:53 AM, Michal Hocko  wrote:
> >
> > That would lead to conflicts when backporting to stable trees though
> > which is quite annoying as well and arguably slightly more annoying than
> > resolving this in mmotm. I can help to rebase Oleg's patch on top of
> > mine which is not a stable material.
> 
> Ok, fair enough - I was actually expecting that Oleg's patch would
> just be marked for stable too just to keep differences minimal.
> 
> But yes, putting your patch in first and then Oleg's on top means that
> it works regardless.
> 
> Any opinions from others?

No pb here, and this one is reasonably easy to backport anyway as the
test is easy to locate.

Willy

Re: [PATCH] mm: mm, mmap: do not blow on PROT_NONE MAP_FIXED holes in the stack

[Linus Torvalds]

On Wed, Jul 5, 2017 at 11:53 AM, Michal Hocko  wrote:
>
> That would lead to conflicts when backporting to stable trees though
> which is quite annoying as well and arguably slightly more annoying than
> resolving this in mmotm. I can help to rebase Oleg's patch on top of
> mine which is not a stable material.

Ok, fair enough - I was actually expecting that Oleg's patch would
just be marked for stable too just to keep differences minimal.

But yes, putting your patch in first and then Oleg's on top means that
it works regardless.

Any opinions from others?

   Linus

Re: [PATCH] mm: mm, mmap: do not blow on PROT_NONE MAP_FIXED holes in the stack

[Linus Torvalds]

On Wed, Jul 5, 2017 at 11:53 AM, Michal Hocko  wrote:
>
> That would lead to conflicts when backporting to stable trees though
> which is quite annoying as well and arguably slightly more annoying than
> resolving this in mmotm. I can help to rebase Oleg's patch on top of
> mine which is not a stable material.

Ok, fair enough - I was actually expecting that Oleg's patch would
just be marked for stable too just to keep differences minimal.

But yes, putting your patch in first and then Oleg's on top means that
it works regardless.

Any opinions from others?

   Linus

Re: [PATCH] mm: mm, mmap: do not blow on PROT_NONE MAP_FIXED holes in the stack

[Michal Hocko]

On Wed 05-07-17 20:53:02, Michal Hocko wrote:
> On Wed 05-07-17 11:35:51, Linus Torvalds wrote:
> > On Wed, Jul 5, 2017 at 11:28 AM, Michal Hocko  wrote:
> > >
> > > Dohh, that was on mmotm which has a clean up by Oleg which reorganizes
> > > the code a bit. This is on top of the current master
> > 
> > Oh, ok. I think I know which patch from Oleg you're talking about.
> > 
> > Since I do want that patch too, and since I'd hate to cause
> > unnecessary merge conflicts in this area, how about we just plan on
> > letting your original patch (on top of Oleg's) go through Andrew and
> > the -mm tree? I'll get it that way, and it's not like this is
> > timing-critical.
> 
> That would lead to conflicts when backporting to stable trees though
> which is quite annoying as well and arguably slightly more annoying than
> resolving this in mmotm. I can help to rebase Oleg's patch on top of
> mine which is not a stable material.

Here is the rebase of Oleg's patch.
---
>From 61ff0cd972dac218390a5859b89ce386db731d1d Mon Sep 17 00:00:00 2001
From: Oleg Nesterov 
Date: Fri, 30 Jun 2017 10:19:00 +0200
Subject: [PATCH] mm/mmap.c: expand_downwards: don't require the gap if
 !vm_prev

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.

Link: http://lkml.kernel.org/r/20170628175258.ga24...@redhat.com
Signed-off-by: Oleg Nesterov 
Cc: Hugh Dickins 
Cc: Larry Woodman 
Cc: Linus Torvalds 
Acked-by: Michal Hocko 
Signed-off-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 ece0f6d3a1b5..f30847405cab 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -2316,7 +2316,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;
@@ -2325,15 +2324,12 @@ 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;
prev = vma->vm_prev;
-   if (prev && prev->vm_end > gap_addr &&
+   /* Check that both stack segments have the same anon_vma? */
+   if (prev && !(prev->vm_flags & VM_GROWSDOWN) &&
(prev->vm_flags & (VM_WRITE|VM_READ|VM_EXEC))) {
-   if (!(prev->vm_flags & VM_GROWSDOWN))
+   if (address - prev->vm_end < stack_guard_gap)
return -ENOMEM;
-   /* Check that both stack segments have the same anon_vma? */
}
 
/* We must make sure the anon_vma is allocated. */
-- 
2.11.0

-- 
Michal Hocko
SUSE Labs

Re: [PATCH] mm: mm, mmap: do not blow on PROT_NONE MAP_FIXED holes in the stack

[Michal Hocko]

On Wed 05-07-17 20:53:02, Michal Hocko wrote:
> On Wed 05-07-17 11:35:51, Linus Torvalds wrote:
> > On Wed, Jul 5, 2017 at 11:28 AM, Michal Hocko  wrote:
> > >
> > > Dohh, that was on mmotm which has a clean up by Oleg which reorganizes
> > > the code a bit. This is on top of the current master
> > 
> > Oh, ok. I think I know which patch from Oleg you're talking about.
> > 
> > Since I do want that patch too, and since I'd hate to cause
> > unnecessary merge conflicts in this area, how about we just plan on
> > letting your original patch (on top of Oleg's) go through Andrew and
> > the -mm tree? I'll get it that way, and it's not like this is
> > timing-critical.
> 
> That would lead to conflicts when backporting to stable trees though
> which is quite annoying as well and arguably slightly more annoying than
> resolving this in mmotm. I can help to rebase Oleg's patch on top of
> mine which is not a stable material.

Here is the rebase of Oleg's patch.
---
>From 61ff0cd972dac218390a5859b89ce386db731d1d Mon Sep 17 00:00:00 2001
From: Oleg Nesterov 
Date: Fri, 30 Jun 2017 10:19:00 +0200
Subject: [PATCH] mm/mmap.c: expand_downwards: don't require the gap if
 !vm_prev

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.

Link: http://lkml.kernel.org/r/20170628175258.ga24...@redhat.com
Signed-off-by: Oleg Nesterov 
Cc: Hugh Dickins 
Cc: Larry Woodman 
Cc: Linus Torvalds 
Acked-by: Michal Hocko 
Signed-off-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 ece0f6d3a1b5..f30847405cab 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -2316,7 +2316,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;
@@ -2325,15 +2324,12 @@ 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;
prev = vma->vm_prev;
-   if (prev && prev->vm_end > gap_addr &&
+   /* Check that both stack segments have the same anon_vma? */
+   if (prev && !(prev->vm_flags & VM_GROWSDOWN) &&
(prev->vm_flags & (VM_WRITE|VM_READ|VM_EXEC))) {
-   if (!(prev->vm_flags & VM_GROWSDOWN))
+   if (address - prev->vm_end < stack_guard_gap)
return -ENOMEM;
-   /* Check that both stack segments have the same anon_vma? */
}
 
/* We must make sure the anon_vma is allocated. */
-- 
2.11.0

-- 
Michal Hocko
SUSE Labs

Re: [PATCH] mm: mm, mmap: do not blow on PROT_NONE MAP_FIXED holes in the stack

[Michal Hocko]

On Wed 05-07-17 11:35:51, Linus Torvalds wrote:
> On Wed, Jul 5, 2017 at 11:28 AM, Michal Hocko  wrote:
> >
> > Dohh, that was on mmotm which has a clean up by Oleg which reorganizes
> > the code a bit. This is on top of the current master
> 
> Oh, ok. I think I know which patch from Oleg you're talking about.
> 
> Since I do want that patch too, and since I'd hate to cause
> unnecessary merge conflicts in this area, how about we just plan on
> letting your original patch (on top of Oleg's) go through Andrew and
> the -mm tree? I'll get it that way, and it's not like this is
> timing-critical.

That would lead to conflicts when backporting to stable trees though
which is quite annoying as well and arguably slightly more annoying than
resolving this in mmotm. I can help to rebase Oleg's patch on top of
mine which is not a stable material.
-- 
Michal Hocko
SUSE Labs

Re: [PATCH] mm: mm, mmap: do not blow on PROT_NONE MAP_FIXED holes in the stack

[Michal Hocko]

On Wed 05-07-17 11:35:51, Linus Torvalds wrote:
> On Wed, Jul 5, 2017 at 11:28 AM, Michal Hocko  wrote:
> >
> > Dohh, that was on mmotm which has a clean up by Oleg which reorganizes
> > the code a bit. This is on top of the current master
> 
> Oh, ok. I think I know which patch from Oleg you're talking about.
> 
> Since I do want that patch too, and since I'd hate to cause
> unnecessary merge conflicts in this area, how about we just plan on
> letting your original patch (on top of Oleg's) go through Andrew and
> the -mm tree? I'll get it that way, and it's not like this is
> timing-critical.

That would lead to conflicts when backporting to stable trees though
which is quite annoying as well and arguably slightly more annoying than
resolving this in mmotm. I can help to rebase Oleg's patch on top of
mine which is not a stable material.
-- 
Michal Hocko
SUSE Labs

Re: [PATCH] mm: mm, mmap: do not blow on PROT_NONE MAP_FIXED holes in the stack

[Linus Torvalds]

On Wed, Jul 5, 2017 at 11:28 AM, Michal Hocko  wrote:
>
> Dohh, that was on mmotm which has a clean up by Oleg which reorganizes
> the code a bit. This is on top of the current master

Oh, ok. I think I know which patch from Oleg you're talking about.

Since I do want that patch too, and since I'd hate to cause
unnecessary merge conflicts in this area, how about we just plan on
letting your original patch (on top of Oleg's) go through Andrew and
the -mm tree? I'll get it that way, and it's not like this is
timing-critical.

Having to fix up conflicts in this area would just be annoying, and
would be nasty for back-porting too.

Linus

Re: [PATCH] mm: mm, mmap: do not blow on PROT_NONE MAP_FIXED holes in the stack

[Linus Torvalds]

On Wed, Jul 5, 2017 at 11:28 AM, Michal Hocko  wrote:
>
> Dohh, that was on mmotm which has a clean up by Oleg which reorganizes
> the code a bit. This is on top of the current master

Oh, ok. I think I know which patch from Oleg you're talking about.

Since I do want that patch too, and since I'd hate to cause
unnecessary merge conflicts in this area, how about we just plan on
letting your original patch (on top of Oleg's) go through Andrew and
the -mm tree? I'll get it that way, and it's not like this is
timing-critical.

Having to fix up conflicts in this area would just be annoying, and
would be nasty for back-porting too.

Linus

Re: [PATCH] mm: mm, mmap: do not blow on PROT_NONE MAP_FIXED holes in the stack

[Michal Hocko]

On Wed 05-07-17 10:43:27, Linus Torvalds wrote:
> On Wed, Jul 5, 2017 at 9:56 AM, Michal Hocko  wrote:
> >
> > "mm: enlarge stack guard gap" has introduced a regression in some rust
> > and Java environments which are trying to implement their own stack
> > guard page.  They are punching a new MAP_FIXED mapping inside the
> > existing stack Vma.
> 
> Hmm. What version is this patch against? It doesn't seem to match my 4.12 
> tree.

Dohh, that was on mmotm which has a clean up by Oleg which reorganizes
the code a bit. This is on top of the current master
---
>From fd538009ac373a5f87538786412a3e6191fa6001 Mon Sep 17 00:00:00 2001
From: Michal Hocko 
Date: Tue, 4 Jul 2017 11:27:39 +0200
Subject: [PATCH] mm: mm, mmap: do not blow on PROT_NONE MAP_FIXED holes in the
 stack

"mm: enlarge stack guard gap" has introduced a regression in some rust
and Java environments which are trying to implement their own stack
guard page.  They are punching a new MAP_FIXED mapping inside the
existing stack Vma.

This will confuse expand_{downwards,upwards} into thinking that the stack
expansion would in fact get us too close to an existing non-stack vma
which is a correct behavior wrt. safety. It is a real regression on
the other hand. Let's work around the problem by considering PROT_NONE
mapping as a part of the stack. This is a gros hack but overflowing to
such a mapping would trap anyway an we only can hope that usespace
knows what it is doing and handle it propely.

Fixes: d4d2d35e6ef9 ("mm: larger stack guard gap, between vmas")
Debugged-by: Vlastimil Babka 
Signed-off-by: Michal Hocko 
---
 mm/mmap.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/mm/mmap.c b/mm/mmap.c
index a5e3dcd75e79..ece0f6d3a1b5 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -2244,7 +2244,8 @@ int expand_upwards(struct vm_area_struct *vma, unsigned 
long address)
gap_addr = TASK_SIZE;
 
next = vma->vm_next;
-   if (next && next->vm_start < gap_addr) {
+   if (next && next->vm_start < gap_addr &&
+   (next->vm_flags & (VM_WRITE|VM_READ|VM_EXEC))) {
if (!(next->vm_flags & VM_GROWSUP))
return -ENOMEM;
/* Check that both stack segments have the same anon_vma? */
@@ -2328,7 +2329,8 @@ int expand_downwards(struct vm_area_struct *vma,
if (gap_addr > address)
return -ENOMEM;
prev = vma->vm_prev;
-   if (prev && prev->vm_end > gap_addr) {
+   if (prev && prev->vm_end > gap_addr &&
+   (prev->vm_flags & (VM_WRITE|VM_READ|VM_EXEC))) {
if (!(prev->vm_flags & VM_GROWSDOWN))
return -ENOMEM;
/* Check that both stack segments have the same anon_vma? */
-- 
2.11.0

Re: [PATCH] mm: mm, mmap: do not blow on PROT_NONE MAP_FIXED holes in the stack

[Michal Hocko]

On Wed 05-07-17 10:43:27, Linus Torvalds wrote:
> On Wed, Jul 5, 2017 at 9:56 AM, Michal Hocko  wrote:
> >
> > "mm: enlarge stack guard gap" has introduced a regression in some rust
> > and Java environments which are trying to implement their own stack
> > guard page.  They are punching a new MAP_FIXED mapping inside the
> > existing stack Vma.
> 
> Hmm. What version is this patch against? It doesn't seem to match my 4.12 
> tree.

Dohh, that was on mmotm which has a clean up by Oleg which reorganizes
the code a bit. This is on top of the current master
---
>From fd538009ac373a5f87538786412a3e6191fa6001 Mon Sep 17 00:00:00 2001
From: Michal Hocko 
Date: Tue, 4 Jul 2017 11:27:39 +0200
Subject: [PATCH] mm: mm, mmap: do not blow on PROT_NONE MAP_FIXED holes in the
 stack

"mm: enlarge stack guard gap" has introduced a regression in some rust
and Java environments which are trying to implement their own stack
guard page.  They are punching a new MAP_FIXED mapping inside the
existing stack Vma.

This will confuse expand_{downwards,upwards} into thinking that the stack
expansion would in fact get us too close to an existing non-stack vma
which is a correct behavior wrt. safety. It is a real regression on
the other hand. Let's work around the problem by considering PROT_NONE
mapping as a part of the stack. This is a gros hack but overflowing to
such a mapping would trap anyway an we only can hope that usespace
knows what it is doing and handle it propely.

Fixes: d4d2d35e6ef9 ("mm: larger stack guard gap, between vmas")
Debugged-by: Vlastimil Babka 
Signed-off-by: Michal Hocko 
---
 mm/mmap.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/mm/mmap.c b/mm/mmap.c
index a5e3dcd75e79..ece0f6d3a1b5 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -2244,7 +2244,8 @@ int expand_upwards(struct vm_area_struct *vma, unsigned 
long address)
gap_addr = TASK_SIZE;
 
next = vma->vm_next;
-   if (next && next->vm_start < gap_addr) {
+   if (next && next->vm_start < gap_addr &&
+   (next->vm_flags & (VM_WRITE|VM_READ|VM_EXEC))) {
if (!(next->vm_flags & VM_GROWSUP))
return -ENOMEM;
/* Check that both stack segments have the same anon_vma? */
@@ -2328,7 +2329,8 @@ int expand_downwards(struct vm_area_struct *vma,
if (gap_addr > address)
return -ENOMEM;
prev = vma->vm_prev;
-   if (prev && prev->vm_end > gap_addr) {
+   if (prev && prev->vm_end > gap_addr &&
+   (prev->vm_flags & (VM_WRITE|VM_READ|VM_EXEC))) {
if (!(prev->vm_flags & VM_GROWSDOWN))
return -ENOMEM;
/* Check that both stack segments have the same anon_vma? */
-- 
2.11.0

Re: [PATCH] mm: mm, mmap: do not blow on PROT_NONE MAP_FIXED holes in the stack

[Linus Torvalds]

On Wed, Jul 5, 2017 at 9:56 AM, Michal Hocko  wrote:
>
> "mm: enlarge stack guard gap" has introduced a regression in some rust
> and Java environments which are trying to implement their own stack
> guard page.  They are punching a new MAP_FIXED mapping inside the
> existing stack Vma.

Hmm. What version is this patch against? It doesn't seem to match my 4.12 tree.

 Linus

Re: [PATCH] mm: mm, mmap: do not blow on PROT_NONE MAP_FIXED holes in the stack

[Linus Torvalds]

On Wed, Jul 5, 2017 at 9:56 AM, Michal Hocko  wrote:
>
> "mm: enlarge stack guard gap" has introduced a regression in some rust
> and Java environments which are trying to implement their own stack
> guard page.  They are punching a new MAP_FIXED mapping inside the
> existing stack Vma.

Hmm. What version is this patch against? It doesn't seem to match my 4.12 tree.

 Linus

[PATCH] mm: mm, mmap: do not blow on PROT_NONE MAP_FIXED holes in the stack

[Michal Hocko]

From: Michal Hocko 

"mm: enlarge stack guard gap" has introduced a regression in some rust
and Java environments which are trying to implement their own stack
guard page.  They are punching a new MAP_FIXED mapping inside the
existing stack Vma.

This will confuse expand_{downwards,upwards} into thinking that the stack
expansion would in fact get us too close to an existing non-stack vma
which is a correct behavior wrt. safety. It is a real regression on
the other hand. Let's work around the problem by considering PROT_NONE
mapping as a part of the stack. This is a gros hack but overflowing to
such a mapping would trap anyway an we only can hope that usespace
knows what it is doing and handle it propely.

Fixes: d4d2d35e6ef9 ("mm: larger stack guard gap, between vmas")
Debugged-by: Vlastimil Babka 
Cc: stable
Signed-off-by: Michal Hocko 
---
Hi,
the original thread [1] has grown quite large and also a bit confusing.
At least the rust part should be fixed by this patch. 32b java will
probably need something more on top of this. Btw. JNI environments rely
on MAP_FIXED PROT_NONE as well they were just lucky to not hit the issue
yet I guess.

[1] http://lkml.kernel.org/r/1499126133.2707.20.ca...@decadent.org.uk
 mm/mmap.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/mm/mmap.c b/mm/mmap.c
index f60a8bc2869c..2e996cbf4ff3 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -2244,7 +2244,8 @@ int expand_upwards(struct vm_area_struct *vma, unsigned 
long address)
gap_addr = TASK_SIZE;
 
next = vma->vm_next;
-   if (next && next->vm_start < gap_addr) {
+   if (next && next->vm_start < gap_addr &&
+   (next->vm_flags & (VM_WRITE|VM_READ|VM_EXEC))) {
if (!(next->vm_flags & VM_GROWSUP))
return -ENOMEM;
/* Check that both stack segments have the same anon_vma? */
@@ -2325,7 +2326,8 @@ int expand_downwards(struct vm_area_struct *vma,
/* Enforce stack_guard_gap */
prev = vma->vm_prev;
/* Check that both stack segments have the same anon_vma? */
-   if (prev && !(prev->vm_flags & VM_GROWSDOWN)) {
+   if (prev && !(prev->vm_flags & VM_GROWSDOWN) &&
+   (prev->vm_flags & (VM_WRITE|VM_READ|VM_EXEC))) {
if (address - prev->vm_end < stack_guard_gap)
return -ENOMEM;
}
-- 
2.11.0

[PATCH] mm: mm, mmap: do not blow on PROT_NONE MAP_FIXED holes in the stack

[Michal Hocko]

From: Michal Hocko 

"mm: enlarge stack guard gap" has introduced a regression in some rust
and Java environments which are trying to implement their own stack
guard page.  They are punching a new MAP_FIXED mapping inside the
existing stack Vma.

This will confuse expand_{downwards,upwards} into thinking that the stack
expansion would in fact get us too close to an existing non-stack vma
which is a correct behavior wrt. safety. It is a real regression on
the other hand. Let's work around the problem by considering PROT_NONE
mapping as a part of the stack. This is a gros hack but overflowing to
such a mapping would trap anyway an we only can hope that usespace
knows what it is doing and handle it propely.

Fixes: d4d2d35e6ef9 ("mm: larger stack guard gap, between vmas")
Debugged-by: Vlastimil Babka 
Cc: stable
Signed-off-by: Michal Hocko 
---
Hi,
the original thread [1] has grown quite large and also a bit confusing.
At least the rust part should be fixed by this patch. 32b java will
probably need something more on top of this. Btw. JNI environments rely
on MAP_FIXED PROT_NONE as well they were just lucky to not hit the issue
yet I guess.

[1] http://lkml.kernel.org/r/1499126133.2707.20.ca...@decadent.org.uk
 mm/mmap.c | 6 --
 1 file changed, 4 insertions(+), 2 deletions(-)

diff --git a/mm/mmap.c b/mm/mmap.c
index f60a8bc2869c..2e996cbf4ff3 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -2244,7 +2244,8 @@ int expand_upwards(struct vm_area_struct *vma, unsigned 
long address)
gap_addr = TASK_SIZE;
 
next = vma->vm_next;
-   if (next && next->vm_start < gap_addr) {
+   if (next && next->vm_start < gap_addr &&
+   (next->vm_flags & (VM_WRITE|VM_READ|VM_EXEC))) {
if (!(next->vm_flags & VM_GROWSUP))
return -ENOMEM;
/* Check that both stack segments have the same anon_vma? */
@@ -2325,7 +2326,8 @@ int expand_downwards(struct vm_area_struct *vma,
/* Enforce stack_guard_gap */
prev = vma->vm_prev;
/* Check that both stack segments have the same anon_vma? */
-   if (prev && !(prev->vm_flags & VM_GROWSDOWN)) {
+   if (prev && !(prev->vm_flags & VM_GROWSDOWN) &&
+   (prev->vm_flags & (VM_WRITE|VM_READ|VM_EXEC))) {
if (address - prev->vm_end < stack_guard_gap)
return -ENOMEM;
}
-- 
2.11.0