Re: [PATCH v2 0/2] mm/mremap: fix two issues with MREMAP_DONTUNMAP

[Andrew Morton]

On Wed, 30 Sep 2026 19:47:08 +0100 "Lorenzo Stoakes (ARM)" <ljs@xxxxxxxxxx> wrote:

> The MREMAP_DONTUNMAP feature is highly unusual in that it permits mremap()
> operations that keep the original VMA in place.
> 
> Historically this has led to a lot of bugs where non-obvious interactions
> occur between existing mremap() operations and the original VMA.
> 
> Commit 397432cab17b ("mm/mremap: account mm->locked_vm correctly for
> MREMAP_DONTUNMAP") fixed an accidentally introduced bug around
> mm->locked_vm accounting, but this wasn't the only issue.
> 
> And thus history repeats itself, as it turns out that mm->locked_vm
> accounting is broken by MREMAP_DONTUNMAP yet again by two further cases,
> and has been broken ever since the feature was introduced.
> 
> Both relate to the fact that VMA_LOCKED_BIT is cleared on the source
> VMA (it has to be as all page tables are moved):
> 
> 1. If an unfaulted VMA_LOCKONFAULT_BIT anonymous VMA self-merges it
>    clears the VMA_LOCKED_BIT flag and permanently leaks mm->locked_vm
>    pages.
> 
> 2. If a partial mremap() is performed on a locked VMA there is a leak equal
>    to the number of pages not copied.
> 
> (Both for MREMAP_DONTUNMAP operations only)
> 
> Both issues can be fixed by treating the source range as distinct from the
> destination range, which is the definition of what MREMAP_DONTUNMAP does so
> is appropriate.

Thanks, updated.

> v2:
> * Added tags (thanks everybody!)
> * Updated 2/2 to avoid splitting the VMA if the VMA was not mlock()'d. It
>   is only meaningful and necessary to perform the split in this case. This
>   also fixes the proc_maps_race selftests that broke, as reported by
>   Anirudh.

Here's how v2 altered mm.git's mm-hotfixes-unstable branch.  Quite a
large change - are you sure that retaining the tags was appropriate?


 mm/mremap.c |   57 +++++++++++++++++++++++++++++---------------------
 mm/vma.c    |    2 -
 2 files changed, 35 insertions(+), 24 deletions(-)

--- a/mm/mremap.c~b
+++ a/mm/mremap.c
@@ -1037,7 +1037,7 @@ static void vrm_stat_account(struct vma_
 }
 
 static bool __check_map_count_against_split(struct mm_struct *mm,
-					    bool is_dontunmap,
+					    bool pre_split,
 					    bool before_unmaps)
 {
 	const int sys_map_count = get_sysctl_max_map_count();
@@ -1091,31 +1091,35 @@ static bool __check_map_count_against_sp
 	 */
 	map_count += 2;
 
-	/*
-	 * If MREMAP_DONTUNMAP is set and a partial operation is performed,
-	 * the VMA is split ahead of time and the -1 observed above doesn't
-	 * apply.
-	 */
-	if (is_dontunmap)
+	/* If pre-split, the -1 observed above doesn't apply. */
+	if (pre_split)
 		map_count++;
 
 	return map_count <= sys_map_count;
 }
 
+static bool needs_pre_split(struct vma_remap_struct *vrm)
+{
+	/*
+	 * An MREMAP_DONTUNMAP of a mlock()'d VMA needs to unlock the
+	 * source VMA, so split in this case.
+	 */
+	return (vrm->flags & MREMAP_DONTUNMAP) &&
+		vma_test(vrm->vma, VMA_LOCKED_BIT);
+}
+
 /* Do we violate the map count limit if we split VMAs when moving the VMA? */
 static bool check_map_count_against_split(struct vma_remap_struct *vrm)
 {
 	return __check_map_count_against_split(current->mm,
-					       vrm->flags & MREMAP_DONTUNMAP,
-					       /*before_unmaps=*/false);
+		needs_pre_split(vrm), /*before_unmaps=*/false);
 }
 
 /* Do we violate the map count limit if we split VMAs prior to early unmaps? */
 static bool check_map_count_against_split_early(struct vma_remap_struct *vrm)
 {
 	return __check_map_count_against_split(current->mm,
-					       vrm->flags & MREMAP_DONTUNMAP,
-					       /*before_unmaps=*/true);
+		vrm->flags & MREMAP_DONTUNMAP, /*before_unmaps=*/true);
 }
 
 /*
@@ -1159,9 +1163,9 @@ static unsigned long prep_move_vma(struc
 
 	/*
 	 * To account mlock()'d pages correctly in the MREMAP_DONTUNMAP
-	 * case perform any split ahead of time.
+	 * case perform any split ahead of time for an mlock()'d VMA.
 	 */
-	if (vrm->flags & MREMAP_DONTUNMAP) {
+	if (needs_pre_split(vrm)) {
 		VMA_ITERATOR(vmi, vma->vm_mm, old_addr);
 
 		if (split_before)
@@ -1356,8 +1360,11 @@ static int copy_vma_and_data(struct vma_
 static void dontunmap_complete(struct vma_remap_struct *vrm,
 			       struct vm_area_struct *new_vma)
 {
+	unsigned long start = vrm->addr;
+	unsigned long end = vrm->addr + vrm->old_len;
 	struct vm_area_struct *vma = vrm->vma;
-	const pgoff_t pgoff_unfaulted = vma->vm_start >> PAGE_SHIFT;
+	unsigned long old_start = vma->vm_start;
+	unsigned long old_end = vma->vm_end;
 
 	/* Self-merge is disallowed. */
 	VM_WARN_ON_ONCE(new_vma == vma);
@@ -1369,15 +1376,19 @@ static void dontunmap_complete(struct vm
 	 * anon_vma links of the old vma is no longer needed after its page
 	 * table has been moved.
 	 */
-	unlink_anon_vmas(vma);
-	/*
-	 * The VMA is now unfaulted and it is an invariant that
-	 * unfaulted anonymous VMAs have page offset equal to
-	 * vma->vm_start >> PAGE_SHIFT.
-	 */
-	vma_set_anon_pgoff(vma, pgoff_unfaulted);
-	if (vma_is_anonymous(vma) && !vma->vm_file)
-		vma_set_pgoff(vma, pgoff_unfaulted);
+	if (start == old_start && end == old_end) {
+		const pgoff_t pgoff_unfaulted = vma->vm_start >> PAGE_SHIFT;
+
+		unlink_anon_vmas(vma);
+		/*
+		 * The VMA is now unfaulted and it is an invariant that
+		 * unfaulted anonymous VMAs have page offset equal to
+		 * vma->vm_start >> PAGE_SHIFT.
+		 */
+		vma_set_anon_pgoff(vma, pgoff_unfaulted);
+		if (vma_is_anonymous(vma) && !vma->vm_file)
+			vma_set_pgoff(vma, pgoff_unfaulted);
+	}
 }
 
 static unsigned long move_vma(struct vma_remap_struct *vrm)
--- a/mm/vma.c~b
+++ a/mm/vma.c
@@ -635,7 +635,7 @@ out_free_vma:
  * either for the first part or the tail.
  */
 int split_vma(struct vma_iterator *vmi, struct vm_area_struct *vma,
-		     unsigned long addr, int new_below)
+	      unsigned long addr, int new_below)
 {
 	if (vma->vm_mm->map_count >= get_sysctl_max_map_count())
 		return -ENOMEM;
_