linux-kernel - Re: [PATCH] mm: mm, mmap: do not blow on PROT_NONE MAP

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 20:28:49 +0200
From:   Michal Hocko <mhocko@...nel.org>
To:     Linus Torvalds <torvalds@...ux-foundation.org>
Cc:     Andrew Morton <akpm@...ux-foundation.org>,
        Vlastimil Babka <vbabka@...e.cz>,
        Ben Hutchings <ben@...adent.org.uk>, Willy Tarreau <w@....eu>,
        Oleg Nesterov <oleg@...hat.com>,
        Rik van Riel <riel@...hat.com>,
        LKML <linux-kernel@...r.kernel.org>,
        linux-mm <linux-mm@...ck.org>
Subject: Re: [PATCH] mm: mm, mmap: do not blow on PROT_NONE MAP_FIXED holes
 in the stack

On Wed 05-07-17 10:43:27, Linus Torvalds wrote:
> On Wed, Jul 5, 2017 at 9:56 AM, Michal Hocko <mhocko@...nel.org> 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 <mhocko@...e.com>
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 <vbabka@...e.cz>
Signed-off-by: Michal Hocko <mhocko@...e.com>
---
 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