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

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:   Wed, 5 Jul 2017 17:58:45 +0100
From:   Ben Hutchings <ben@...adent.org.uk>
To:     Michal Hocko <mhocko@...nel.org>
Cc:     Linus Torvalds <torvalds@...ux-foundation.org>,
        Willy Tarreau <w@....eu>, Hugh Dickins <hughd@...gle.com>,
        Oleg Nesterov <oleg@...hat.com>,
        "Jason A. Donenfeld" <Jason@...c4.com>,
        Rik van Riel <riel@...hat.com>,
        Larry Woodman <lwoodman@...hat.com>,
        "Kirill A. Shutemov" <kirill@...temov.name>,
        Tony Luck <tony.luck@...el.com>,
        "James E.J. Bottomley" <jejb@...isc-linux.org>,
        Helge Diller <deller@....de>,
        James Hogan <james.hogan@...tec.com>,
        Laura Abbott <labbott@...hat.com>, Greg KH <greg@...ah.com>,
        "security@...nel.org" <security@...nel.org>,
        Qualys Security Advisory <qsa@...lys.com>,
        LKML <linux-kernel@...r.kernel.org>,
        Ximin Luo <infinity0@...ian.org>
Subject: Re: [PATCH] mm: larger stack guard gap, between vmas

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@...adent.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@...adent.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.

Download attachment "signature.asc" of type "application/pgp-signature" (812 bytes)