linux-kernel - Re: [PATCH] Fix OOPS in mmap_region() when merging adjacent VM

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]

Message-ID: <alpine.LFD.2.00.0901291228250.3123@localhost.localdomain>
Date:	Thu, 29 Jan 2009 12:33:50 -0800 (PST)
From:	Linus Torvalds <torvalds@...ux-foundation.org>
To:	Lee Schermerhorn <Lee.Schermerhorn@...com>
cc:	linux-kernel <linux-kernel@...r.kernel.org>,
	Maksim Yevmenkin <maksim.yevmenkin@...il.com>,
	Nick Piggin <npiggin@...e.de>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Greg Kroah-Hartman <gregkh@...e.de>, will@...wder-design.com,
	Rik van Riel <riel@...hat.com>,
	KOSAKI Motohiro <kosaki.motohiro@...fujitsu.com>,
	KAMEZAWA Hiroyuki <kamezawa.hiroyu@...fujitsu.com>
Subject: Re: [PATCH] Fix OOPS in mmap_region() when merging adjacent VM_LOCKED
 file segments



On Thu, 29 Jan 2009, Lee Schermerhorn wrote:
> 
> This patch provides a simplistic fix, suitable, I hope, for -stable.

So I've looked a bit at that function, and I wonder why we can't just do 
the "vma_merge()" earlier - before we allocate that whole vma to begin 
with.

We already do that for anonymous mappings _anyway_, using the exact same 
"vma_merge()" function. So how about just expanding on that logic, and 
simplifying everything at the same time?

THIS PATCH IS TOTALLY UNTESTED!

There may be some reason why we didn't do it this way to begin with, but 
this woudl actually seem to be the more logical way to fix the problem: by 
simplifying the whole function to the point where we never try to do that 
"try to merge late and then fix up the fact that we have to free the vma 
we allocated earlier" thing at all!

Hmm? This would remove a more lines than it adds, and actually makes 
things more readable. If it works. Anybody see why it wouldn't?

		Linus

---
 mm/mmap.c |   30 ++++++++++--------------------
 1 files changed, 10 insertions(+), 20 deletions(-)

diff --git a/mm/mmap.c b/mm/mmap.c
index 8d95902..3f78ead 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -769,6 +769,10 @@ struct vm_area_struct *vma_merge(struct mm_struct *mm,
 	if (vm_flags & VM_SPECIAL)
 		return NULL;
 
+	/* Anonymous shared mappings are unsharable */
+	if ((vm_flags & VM_SHARED) && !file)
+		return NULL;
+
 	if (prev)
 		next = prev->vm_next;
 	else
@@ -1134,16 +1138,11 @@ munmap_back:
 	}
 
 	/*
-	 * Can we just expand an old private anonymous mapping?
-	 * The VM_SHARED test is necessary because shmem_zero_setup
-	 * will create the file object for a shared anonymous map below.
+	 * Can we just expand an old mapping?
 	 */
-	if (!file && !(vm_flags & VM_SHARED)) {
-		vma = vma_merge(mm, prev, addr, addr + len, vm_flags,
-					NULL, NULL, pgoff, NULL);
-		if (vma)
-			goto out;
-	}
+	vma = vma_merge(mm, prev, addr, addr + len, vm_flags, NULL, file, pgoff, NULL);
+	if (vma)
+		goto out;
 
 	/*
 	 * Determine the object being mapped and call the appropriate
@@ -1206,17 +1205,8 @@ munmap_back:
 	if (vma_wants_writenotify(vma))
 		vma->vm_page_prot = vm_get_page_prot(vm_flags & ~VM_SHARED);
 
-	if (file && vma_merge(mm, prev, addr, vma->vm_end,
-			vma->vm_flags, NULL, file, pgoff, vma_policy(vma))) {
-		mpol_put(vma_policy(vma));
-		kmem_cache_free(vm_area_cachep, vma);
-		fput(file);
-		if (vm_flags & VM_EXECUTABLE)
-			removed_exe_file_vma(mm);
-	} else {
-		vma_link(mm, vma, prev, rb_link, rb_parent);
-		file = vma->vm_file;
-	}
+	vma_link(mm, vma, prev, rb_link, rb_parent);
+	file = vma->vm_file;
 
 	/* Once vma denies write, undo our temporary denial count */
 	if (correct_wcount)
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/