Thread (90 messages) 90 messages, 9 authors, 19d ago

Re: [PATCH 04/39] mm/vma: ensure mmap_prepare doesn't set actions on a mergeable vma

From: sashiko-bot@kernel.org
Date: 2026-09-08 20:36:43
Also in: bpf, dri-devel, kvm, kvmarm, linux-perf-users, linux-s390, linux-scsi, selinux

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] File reference leak in error paths of `call_mmap_prepare()` and `__compat_vma_mmap()` when validation fails after file replacement.
--

commit c2c06e31f69acde90d013dcbd918472d63f3996c
Author: Lorenzo Stoakes (ARM) [off-list ref]

mm/vma: ensure mmap_prepare doesn't set actions on a mergeable vma

This commit adds validation to mmap_prepare_validate() to ensure that mmap
actions are not set on VMAs that can be merged. It returns -EINVAL if such an
action is detected.
quoted hunk ↗ jump to hunk
diff --git a/mm/vma.c b/mm/vma.c
index a24f04428580b..9e45fc83f74c6 100644
--- a/mm/vma.c
+++ b/mm/vma.c
@@ -2794,6 +2794,15 @@ static int mmap_validate(unsigned long prev_start,
 int mmap_prepare_validate(const struct vm_area_desc *prev_desc,
 			  const struct vm_area_desc *desc)
 {
+	/*
+	 * It is not valid to execute mmap actions for VMAs which can be merged,
+	 * as any such merge would leave portions of the mapping incorrectly
+	 * unmapped.
+	 */
+	if (vma_flags_can_merge(&desc->vma_flags) &&
+	    WARN_ON_ONCE(desc->action.type != MMAP_NOTHING))
+		return -EINVAL;
+
[Severity: High]
This is a pre-existing issue, but by expanding the validation checks here, does
this increase the surface area of a struct file leak in the error paths of
call_mmap_prepare() and __compat_vma_mmap()?

If a driver's mmap_prepare hook replaces the mapping's backing file, it returns
a new file in desc->vm_file with an elevated refcount.

If this newly added validation step fails, call_mmap_prepare() returns early:

mm/vma.c:call_mmap_prepare() {
	...
	err = mmap_prepare_validate(&prev_desc, desc);
	if (err)
		return err;

This skips the crucial handoff that ensures the cleanup code in abort_munmap()
will drop the reference to the new file:

	if (desc->vm_file != map->file) {
		map->file_doesnt_need_get = true;
		map->file = desc->vm_file;
	}
	...
}

Since map->file and map->file_doesnt_need_get are not updated on error, the
caller's cleanup code ignores the new file reference, leading to a permanent
leak of the struct file.

The same early return leak applies to __compat_vma_mmap() in mm/util.c, where
desc is a stack variable and desc->vm_file is lost on error.

Should these error paths handle fput() on desc->vm_file if it was replaced?
 	return mmap_validate(prev_desc->start, desc->start,
 			     &prev_desc->vma_flags, &desc->vma_flags);
 }
-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260908-b4-mmap-prepare-vma-flag-sanify-v1-0-dacf19cce22b@kernel.org?part=4
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help