mm: nommu: fix the error path when vma_iter_prealloc() fails

When vma_iter_prealloc() fails in do_mmap(), it jumps to error_just_free without updating ret to -ENOMEM, meaning do_mmap() will return 0 on failure.

Additionally, this error path unconditionally frees the region struct. Since the region was already added to the global nommu_region_tree via add_nommu_region(), leaving it makes a potential dangling pointer in the tree and may cause a use-after-free on the next tree walk.

This commit fixes those issues by introducing new jump label, error_vma_iter_prealloc, to correctly handle the error case of vma_iter_prealloc(), and move the region updates after the place that the allocation is finished.

Those issues are discovered by Sashiko, linked below.

---
 mm/nommu.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

index 815ebefddfae..ad619611e83f 100644
@@ -1181,7 +1181,6 @@ unsigned long do_mmap(struct file *file,
 		ret = do_mmap_private(vma, region, len, capabilities);
 	if (ret < 0)
 		goto error_just_free;
-	add_nommu_region(region);
 
 	/* clear anonymous mappings that don't ask for uninitialized data */
 	if (!vma->vm_file &&

@@ -1199,7 +1198,9 @@ unsigned long do_mmap(struct file *file,
 	BUG_ON(!vma->vm_region);
 	vma_iter_config(&vmi, vma->vm_start, vma->vm_end);
 	if (vma_iter_prealloc(&vmi, vma))
-		goto error_just_free;
+		goto error_vma_iter_prealloc;
+
+	add_nommu_region(region);
 
 	setup_vma_to_mm(vma, current->mm);
 	current->mm->map_count++;

@@ -1235,6 +1236,12 @@ unsigned long do_mmap(struct file *file,
 	ret = -EINVAL;
 	goto error;
 
+error_vma_iter_prealloc:
+	up_write(&nommu_region_sem);
+	pr_warn("Failed vma_iter_prealloc()\n");
+	ret = -ENOMEM;
+	goto error;
+
 error_getting_vma:
 	kmem_cache_free(vm_region_jar, region);
 	pr_warn("Allocation of vma for %lu byte allocation from process %d failed\n",

2.43.0

If do_mmap() finds an existing overlapping shared region, it increments its usage, sets region to this existing pregion, and jumps to share:

	region = pregion;
	result = start;
	goto share;

When vma_iter_prealloc() fails and jumps to error_vma_iter_prealloc, the error path unconditionally frees the region:

error:

	...
	if (region->vm_file)
		fput(region->vm_file);
	kmem_cache_free(vm_region_jar, region);

This potentially leaves a dangling pointer in nommu_region_tree and causes RB-tree corruption.

Additionally, when establishing a new private mapping, do_mmap_private() allocates physical pages and assigns them to region->vm_start:

	base = alloc_pages_exact(total << PAGE_SHIFT, GFP_KERNEL);
	...
	region->vm_start = (unsigned long) base;

If we later fail at vma_iter_prealloc() and jump to error_vma_iter_prealloc, the region struct is freed, but the backing physical memory doesn't seem to be freed via free_page_series().

This commit fixes those issues, discovered Sashiko, linked below.

---
 mm/nommu.c | 18 +++++++++++++++---
 1 file changed, 15 insertions(+), 3 deletions(-)

index ad619611e83f..24bd8c529c2a 100644
@@ -1222,9 +1222,16 @@ unsigned long do_mmap(struct file *file,
 	up_write(&nommu_region_sem);
 error:
 	vma_iter_free(&vmi);
-	if (region->vm_file)
-		fput(region->vm_file);
-	kmem_cache_free(vm_region_jar, region);
+
+	/* if the error was from shared mapping/existing region, don't free the region  */
+	if (region->vm_usage == 1) {
+		if (region->vm_file)
+			fput(region->vm_file);
+		kmem_cache_free(vm_region_jar, region);
+
+	} else
+		region->vm_usage--;
+
 	if (vma->vm_file)
 		fput(vma->vm_file);
 	vm_area_free(vma);

@@ -1240,6 +1247,11 @@ unsigned long do_mmap(struct file *file,
 	up_write(&nommu_region_sem);
 	pr_warn("Failed vma_iter_prealloc()\n");
 	ret = -ENOMEM;
+
+	/* in case that the region is allocated via do_mmap_private() */
+	if ((region->vm_usage == 1) &&
+	    (!file || !(vma->vm_flags & VM_SHARED)))
+		free_page_series(region->vm_start, region->vm_top);
 	goto error;
 
 error_getting_vma:

2.43.0

Thanks. Now I'm wondering if you regret opening this can of worms ;)

	https://sashiko.dev/#/patchset/[email protected]

afaict, all these issues pertain to failures from vma_iter_prealloc().

indeed, this is why I wished to do it locally to avoid this noise (but failed/struggled to prepare the local sashiko due to lack of subscription for me..).

I'm sorry for the noise but this is what I wanted to know so, no problem for me.

yes, will look into detail and get you back with another spin.

-- Hajime

now with two more spins, Sashiko reports no issues with v3 patch.

https://sashiko.dev/#/patchset/20260708083829.576036-1-thehajime%40gmail.com

-- Hajime