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

Ben Hutchings
2017-07-05T16:59:17Z

[-- Attachment #1: Type: text/plain, Size: 3899 bytes --]

On Wed, Jul 05, 2017 at 04:25:00PM +0100, Ben Hutchings wrote:
[...]
> Soemthing I noticed is that Java doesn't immediately use MAP_FIXED.
> Look at os::pd_attempt_reserve_memory_at(). If the first, hinted,
> mmap() doesn't return the hinted address it then attempts to allocate
> huge areas (I'm not sure how intentional this is) and unmaps the
> unwanted parts. Then os::workaround_expand_exec_shield_cs_limit() re-
> mmap()s the wanted part with MAP_FIXED. If this fails at any point it
> is not a fatal error.
>
> So if we change vm_start_gap() to take the stack limit into account
> (when it's finite) that should neutralise
> os::workaround_expand_exec_shield_cs_limit(). I'll try this.

I ended up with the following two patches, which seem to deal with
both the Java and Rust regressions. These don't touch the
stack-grows-up paths at all because Rust doesn't run on those
architectures and the Java weirdness is i386-specific.

They definitely need longer commit messages and comments, but aside
from that do these look reasonable?

Ben.

Subject: [1/2] mmap: Skip a single VM_NONE mapping when checking the stack gap

Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
---
mm/mmap.c | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/mm/mmap.c b/mm/mmap.c
index a5e3dcd75e79..c7906ae1a7a1 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -2323,11 +2323,16 @@ int expand_downwards(struct vm_area_struct *vma,
if (error)
return error;

- /* Enforce stack_guard_gap */
+ /*
+ * Enforce stack_guard_gap. Some applications allocate a VM_NONE
+ * mapping just below the stack, which we can safely ignore.
+ */
gap_addr = address - stack_guard_gap;
if (gap_addr > address)
return -ENOMEM;
prev = vma->vm_prev;
+ if (prev && !(prev->vm_flags & (VM_READ | VM_WRITE | VM_EXEC)))
+ prev = prev->vm_prev;
if (prev && prev->vm_end > gap_addr) {
if (!(prev->vm_flags & VM_GROWSDOWN))
return -ENOMEM;

Subject: [2/2] mmap: Avoid mapping anywhere within the full stack extent
if finite

Signed-off-by: Ben Hutchings <ben@decadent.org.uk>
---
include/linux/mm.h | 9 ++++-----
mm/mmap.c | 19 +++++++++++++++++++
2 files changed, 23 insertions(+), 5 deletions(-)

diff --git a/include/linux/mm.h b/include/linux/mm.h
index 6f543a47fc92..2240a0505072 100644
--- a/include/linux/mm.h
+++ b/include/linux/mm.h
@@ -2223,15 +2223,14 @@ static inline struct vm_area_struct * find_vma_intersection(struct mm_struct * m
return vma;
}

+unsigned long __vm_start_gap(struct vm_area_struct *vma);
+
static inline unsigned long vm_start_gap(struct vm_area_struct *vma)
{
unsigned long vm_start = vma->vm_start;

- if (vma->vm_flags & VM_GROWSDOWN) {
- vm_start -= stack_guard_gap;
- if (vm_start > vma->vm_start)
- vm_start = 0;
- }
+ if (vma->vm_flags & VM_GROWSDOWN)
+ vm_start = __vm_start_gap(vma);
return vm_start;
}

diff --git a/mm/mmap.c b/mm/mmap.c
index c7906ae1a7a1..f8131a94e56e 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -2307,6 +2307,25 @@ int expand_upwards(struct vm_area_struct *vma, unsigned long address)
}
#endif /* CONFIG_STACK_GROWSUP || CONFIG_IA64 */

+unsigned long __vm_start_gap(struct vm_area_struct *vma)
+{
+ unsigned long stack_limit =
+ current->signal->rlim[RLIMIT_STACK].rlim_cur;
+ unsigned long vm_start;
+
+ if (stack_limit != RLIM_INFINITY &&
+ vma->vm_end - vma->vm_start < stack_limit)
+ vm_start = vma->vm_end - PAGE_ALIGN(stack_limit);
+ else
+ vm_start = vma->vm_start;
+
+ vm_start -= stack_guard_gap;
+ if (vm_start > vma->vm_start)
+ vm_start = 0;
+
+ return vm_start;
+}
+
/*
* vma is the first one with address < vma->vm_start. Have to extend vma.
*/

--
Ben Hutchings
For every complex problem
there is a solution that is simple, neat, and wrong.

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 811 bytes --]