From: Jeff Xu <jef...@chromium.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 <jef...@chromium.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


Reply via email to