linux-kernel - [PATCH v5.1 17/19] mm/mmap: Move can_modify

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>] [day] [month] [year] [list]

Message-ID: <20240816161011.2825-1-spasswolf@web.de>
Date: Fri, 16 Aug 2024 18:10:09 +0200
From: Bert Karwatzki <spasswolf@....de>
To: "Liam R . Howlett" <Liam.Howlett@...cle.com>
Cc: Bert Karwatzki <spasswolf@....de>,
	Suren Baghdasaryan <surenb@...gle.com>,
	Vlastimil Babka <vbabka@...e.cz>,
	Lorenzo Stoakes <lstoakes@...il.com>,
	Matthew Wilcox <willy@...radead.org>,
	sidhartha.kumar@...cle.com,
	"Paul E . McKenney" <paulmck@...nel.org>,
	Jiri Olsa <olsajiri@...il.com>,
	linux-kernel@...r.kernel.org,
	linux-mm@...ck.org,
	Andrew Morton <akpm@...ux-foundation.org>,
	Kees Cook <kees@...nel.org>,
	Jeff Xu <jeffxu@...omium.org>,
	"Liam R . Howlett" <Liam.Howlett@...cle.com>
Subject: [PATCH v5.1 17/19] mm/mmap: Move can_modify_mm() check down the stack

Without an arch_unmap() call so high in the call stack, the check for
mseal'ed vmas can be moved lower as well.  This has the benefit of only
actually checking if things are msealed when there is anything to check.
That is, we know there is at least one vma that is in the way and needs
to be checked.

Only call the can_modify_mm() in do_vmi_align_munmap() and the MAP_FIXED
case of mmap_region().

Signed-off-by: Liam R. Howlett <Liam.Howlett@...cle.com>
Cc: Jeff Xu <jeffxu@...omium.org>
---
 mm/mmap.c | 13 ++++---------
 mm/vma.c  | 11 ++++-------
 2 files changed, 8 insertions(+), 16 deletions(-)

diff --git a/mm/mmap.c b/mm/mmap.c
index d0942b1a62f0..ae74d0674b6d 100644
--- a/mm/mmap.c
+++ b/mm/mmap.c
@@ -1387,13 +1387,15 @@ unsigned long mmap_region(struct file *file, unsigned long addr,
 	if (!may_expand_vm(mm, vm_flags, pglen - nr_pages))
 		return -ENOMEM;

-	if (unlikely(!can_modify_mm(mm, addr, end)))
-		return -EPERM;

 	/* Find the first overlapping VMA */
 	vma = vma_find(&vmi, end);
 	init_vma_munmap(&vms, &vmi, vma, addr, end, uf, /* unlock = */ false);
 	if (vma) {
+		/* Prevent unmapping a sealed VMA. */
+		if (unlikely(!can_modify_mm(mm, addr, end)))
+			return -EPERM;
+
 		mt_init_flags(&mt_detach, vmi.mas.tree->ma_flags & MT_FLAGS_LOCK_MASK);
 		mt_on_stack(mt_detach);
 		mas_init(&mas_detach, &mt_detach, /* addr = */ 0);
@@ -1766,13 +1768,6 @@ int do_vma_munmap(struct vma_iterator *vmi, struct vm_area_struct *vma,
 {
 	struct mm_struct *mm = vma->vm_mm;

-	/*
-	 * Check if memory is sealed, prevent unmapping a sealed VMA.
-	 * can_modify_mm assumes we have acquired the lock on MM.
-	 */
-	if (unlikely(!can_modify_mm(mm, start, end)))
-		return -EPERM;
-
 	return do_vmi_align_munmap(vmi, vma, mm, start, end, uf, unlock);
 }

diff --git a/mm/vma.c b/mm/vma.c
index 86757443a7a2..f267b2552c8b 100644
--- a/mm/vma.c
+++ b/mm/vma.c
@@ -957,6 +957,10 @@ do_vmi_align_munmap(struct vma_iterator *vmi, struct vm_area_struct *vma,
 	struct vma_munmap_struct vms;
 	int error;

+	/* Prevent unmapping a sealed VMA. */
+	if (unlikely(!can_modify_mm(mm, start, end)))
+		return -EPERM;
+
 	init_vma_munmap(&vms, vmi, vma, start, end, uf, unlock);
 	error = vms_gather_munmap_vmas(&vms, &mas_detach);
 	if (error)
@@ -1007,13 +1011,6 @@ int do_vmi_munmap(struct vma_iterator *vmi, struct mm_struct *mm,
 	if (end == start)
 		return -EINVAL;

-	/*
-	 * Check if memory is sealed, prevent unmapping a sealed VMA.
-	 * can_modify_mm assumes we have acquired the lock on MM.
-	 */
-	if (unlikely(!can_modify_mm(mm, start, end)))
-		return -EPERM;
-
 	/* Find the first overlapping VMA */
 	vma = vma_find(vmi, end);
 	if (!vma) {
--
2.45.2