| 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
| ||
|
Message-ID: <20240814071424.2655666-3-jeffxu@chromium.org>
Date: Wed, 14 Aug 2024 07:14:24 +0000
From: jeffxu@...omium.org
To: akpm@...ux-foundation.org,
willy@...radead.org,
torvalds@...ux-foundation.org,
Liam.Howlett@...cle.com,
pedro.falcato@...il.com
Cc: linux-kernel@...r.kernel.org,
linux-kselftest@...r.kernel.org,
linux-mm@...ck.org,
linux-hardening@...r.kernel.org,
jeffxu@...gle.com,
lorenzo.stoakes@...cle.com,
mpe@...erman.id.au,
oliver.sang@...el.com,
vbabka@...e.cz,
keescook@...omium.org,
Jeff Xu <jeffxu@...omium.org>
Subject: [PATCH v1 2/2] mseal: refactor mremap to remove can_modify_mm
From: Jeff Xu <jeffxu@...omium.org>
mremap doesn't allow relocate, expand, shrink across VMA boundaries,
refactor the code to check src address range before doing anything on
the destination.
This also allow we remove can_modify_mm from mremap, since
the src address must be single VMA, use can_modify_vma instead.
Signed-off-by: Jeff Xu <jeffxu@...omium.org>
---
mm/internal.h | 24 ++++++++++++++++
mm/mremap.c | 77 +++++++++++++++++++++++++--------------------------
mm/mseal.c | 17 ------------
3 files changed, 61 insertions(+), 57 deletions(-)
diff --git a/mm/internal.h b/mm/internal.h
index b4d86436565b..53f0bbbc6449 100644
--- a/mm/internal.h
+++ b/mm/internal.h
@@ -1501,6 +1501,24 @@ bool can_modify_mm(struct mm_struct *mm, unsigned long start,
unsigned long end);
bool can_modify_mm_madv(struct mm_struct *mm, unsigned long start,
unsigned long end, int behavior);
+
+static inline bool vma_is_sealed(struct vm_area_struct *vma)
+{
+ return (vma->vm_flags & VM_SEALED);
+}
+
+/*
+ * check if a vma is sealed for modification.
+ * return true, if modification is allowed.
+ */
+static inline bool can_modify_vma(struct vm_area_struct *vma)
+{
+ if (unlikely(vma_is_sealed(vma)))
+ return false;
+
+ return true;
+}
+
#else
static inline int can_do_mseal(unsigned long flags)
{
@@ -1518,6 +1536,12 @@ static inline bool can_modify_mm_madv(struct mm_struct *mm, unsigned long start,
{
return true;
}
+
+static inline bool can_modify_vma(struct vm_area_struct *vma)
+{
+ return true;
+}
+
#endif
#ifdef CONFIG_SHRINKER_DEBUG
diff --git a/mm/mremap.c b/mm/mremap.c
index e7ae140fc640..3c5bb671a280 100644
--- a/mm/mremap.c
+++ b/mm/mremap.c
@@ -904,28 +904,7 @@ static unsigned long mremap_to(unsigned long addr, unsigned long old_len,
/*
* In mremap_to().
- * Move a VMA to another location, check if src addr is sealed.
- *
- * Place can_modify_mm here because mremap_to()
- * does its own checking for address range, and we only
- * check the sealing after passing those checks.
- *
- * can_modify_mm assumes we have acquired the lock on MM.
*/
- if (unlikely(!can_modify_mm(mm, addr, addr + old_len)))
- return -EPERM;
-
- if (flags & MREMAP_FIXED) {
- /*
- * In mremap_to().
- * VMA is moved to dst address, and munmap dst first.
- * do_munmap will check if dst is sealed.
- */
- ret = do_munmap(mm, new_addr, new_len, uf_unmap_early);
- if (ret)
- goto out;
- }
-
if (old_len > new_len) {
ret = do_munmap(mm, addr+new_len, old_len - new_len, uf_unmap);
if (ret)
@@ -939,6 +918,26 @@ static unsigned long mremap_to(unsigned long addr, unsigned long old_len,
goto out;
}
+ /*
+ * Since we can't remap across vma boundaries,
+ * check single vma instead of src address range.
+ */
+ if (unlikely(!can_modify_vma(vma))) {
+ ret = -EPERM;
+ goto out;
+ }
+
+ if (flags & MREMAP_FIXED) {
+ /*
+ * In mremap_to().
+ * VMA is moved to dst address, and munmap dst first.
+ * do_munmap will check if dst is sealed.
+ */
+ ret = do_munmap(mm, new_addr, new_len, uf_unmap_early);
+ if (ret)
+ goto out;
+ }
+
/* MREMAP_DONTUNMAP expands by old_len since old_len == new_len */
if (flags & MREMAP_DONTUNMAP &&
!may_expand_vm(mm, vma->vm_flags, old_len >> PAGE_SHIFT)) {
@@ -1079,19 +1078,6 @@ SYSCALL_DEFINE5(mremap, unsigned long, addr, unsigned long, old_len,
goto out;
}
- /*
- * Below is shrink/expand case (not mremap_to())
- * Check if src address is sealed, if so, reject.
- * In other words, prevent shrinking or expanding a sealed VMA.
- *
- * Place can_modify_mm here so we can keep the logic related to
- * shrink/expand together.
- */
- if (unlikely(!can_modify_mm(mm, addr, addr + old_len))) {
- ret = -EPERM;
- goto out;
- }
-
/*
* Always allow a shrinking remap: that just unmaps
* the unnecessary pages..
@@ -1107,7 +1093,7 @@ SYSCALL_DEFINE5(mremap, unsigned long, addr, unsigned long, old_len,
}
ret = do_vmi_munmap(&vmi, mm, addr + new_len, old_len - new_len,
- &uf_unmap, true);
+ &uf_unmap, true);
if (ret)
goto out;
@@ -1124,6 +1110,15 @@ SYSCALL_DEFINE5(mremap, unsigned long, addr, unsigned long, old_len,
goto out;
}
+ /*
+ * Since we can't remap across vma boundaries,
+ * check single vma instead of src address range.
+ */
+ if (unlikely(!can_modify_vma(vma))) {
+ ret = -EPERM;
+ goto out;
+ }
+
/* old_len exactly to the end of the area..
*/
if (old_len == vma->vm_end - addr) {
@@ -1132,9 +1127,10 @@ SYSCALL_DEFINE5(mremap, unsigned long, addr, unsigned long, old_len,
/* can we just expand the current mapping? */
if (vma_expandable(vma, delta)) {
long pages = delta >> PAGE_SHIFT;
- VMA_ITERATOR(vmi, mm, vma->vm_end);
long charged = 0;
+ VMA_ITERATOR(vmi, mm, vma->vm_end);
+
if (vma->vm_flags & VM_ACCOUNT) {
if (security_vm_enough_memory_mm(mm, pages)) {
ret = -ENOMEM;
@@ -1177,20 +1173,21 @@ SYSCALL_DEFINE5(mremap, unsigned long, addr, unsigned long, old_len,
ret = -ENOMEM;
if (flags & MREMAP_MAYMOVE) {
unsigned long map_flags = 0;
+
if (vma->vm_flags & VM_MAYSHARE)
map_flags |= MAP_SHARED;
new_addr = get_unmapped_area(vma->vm_file, 0, new_len,
- vma->vm_pgoff +
- ((addr - vma->vm_start) >> PAGE_SHIFT),
- map_flags);
+ vma->vm_pgoff +
+ ((addr - vma->vm_start) >> PAGE_SHIFT),
+ map_flags);
if (IS_ERR_VALUE(new_addr)) {
ret = new_addr;
goto out;
}
ret = move_vma(vma, addr, old_len, new_len, new_addr,
- &locked, flags, &uf, &uf_unmap);
+ &locked, flags, &uf, &uf_unmap);
}
out:
if (offset_in_page(ret))
diff --git a/mm/mseal.c b/mm/mseal.c
index bf783bba8ed0..4591ae8d29c2 100644
--- a/mm/mseal.c
+++ b/mm/mseal.c
@@ -16,28 +16,11 @@
#include <linux/sched.h>
#include "internal.h"
-static inline bool vma_is_sealed(struct vm_area_struct *vma)
-{
- return (vma->vm_flags & VM_SEALED);
-}
-
static inline void set_vma_sealed(struct vm_area_struct *vma)
{
vm_flags_set(vma, VM_SEALED);
}
-/*
- * check if a vma is sealed for modification.
- * return true, if modification is allowed.
- */
-static bool can_modify_vma(struct vm_area_struct *vma)
-{
- if (unlikely(vma_is_sealed(vma)))
- return false;
-
- return true;
-}
-
static bool is_madv_discard(int behavior)
{
return behavior &
--
2.46.0.76.ge559c4bf1a-goog
Powered by blists - more mailing lists