Re: [PATCH 03/39] mm: consistently validate VMA state after mmap[_prepare] hooks
From: sashiko-bot@kernel.org
Date: 2026-09-08 20:40:34
Also in:
bpf, dri-devel, kvm, kvmarm, linux-s390, linux-scsi, linux-trace-kernel, selinux
Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider: - [High] Error cleanup path unmaps arbitrary memory if a driver illegally alters vma->vm_start. - [High] Driver resources and file reference are leaked when mmap_prepare_validate() fails after a successful hook. -- commit 7871b5bece83247c9fe3efaac4b928710f478390 Author: Lorenzo Stoakes (ARM) [off-list ref] mm: consistently validate VMA state after mmap[_prepare] hooks This commit introduces mmap_prepare_validate() and mmap_hook_validate() to verify that mmap hooks do not perform unauthorized operations, such as modifying the VMA start address. It applies these checks uniformly across VMA creation paths.
quoted hunk ↗ jump to hunk
diff --git a/mm/internal.h b/mm/internal.h --- a/mm/internal.h +++ b/mm/internal.h
[ ... ]
quoted hunk ↗ jump to hunk
@@ -238,26 +259,14 @@ static inline int mmap_file(struct file *file, struct vm_area_struct *vma) */ if (unlikely(err || !vma->vm_ops)) vma->vm_ops = &vma_dummy_vm_ops; + if (unlikely(err)) + return err; + err = mmap_hook_validate(prev_start, &prev_flags, vma); + if (unlikely(err)) + vma_close(vma); + return err; }
[Severity: High]
If a driver illegally alters vma->vm_start, mmap_hook_validate() will
correctly catch the violation and return -EINVAL. However, does this leave
the corrupted vma->vm_start in place when returning to the caller?
Looking at __mmap_new_file_vma(), the error handling path relies on
vma->vm_start:
error = mmap_file(vma->vm_file, vma);
if (error) {
UNMAP_STATE(unmap, vmi, vma, vma->vm_start, vma->vm_end,
map->prev, map->next);
...
unmap_region(&unmap);
return error;
}
If mmap_file() fails because the driver altered vma->vm_start, could
unmap_region() accidentally unmap arbitrary user memory using the corrupted
start address?
quoted hunk ↗ jump to hunk
diff --git a/mm/util.c b/mm/util.c --- a/mm/util.c +++ b/mm/util.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -1224,10 +1224,17 @@ EXPORT_SYMBOL(compat_set_desc_from_vma); int __compat_vma_mmap(struct vm_area_desc *desc, struct vm_area_struct *vma) { + struct vm_area_desc prev_desc; int err; + /* Derive state prior to mmap_prepare hook. */ + compat_set_desc_from_vma(&prev_desc, desc->file, vma); /* Perform any preparatory tasks for mmap action. */ err = mmap_action_prepare(desc); + if (err) + return err; + /* Check the caller did nothing crazy. */ + err = mmap_prepare_validate(&prev_desc, desc); if (err) return err;
[Severity: High] If mmap_prepare_validate() detects an invariant violation and returns an error here, does this early return leak driver resources and the file reference? If the driver's mmap_prepare() hook succeeded, it may have allocated resources (expecting them to be freed later via vm_ops->close) or placed a new file reference in desc->vm_file. Since vma_close() is never called and the local desc structure is discarded, it appears these resources might be leaked.
quoted hunk ↗ jump to hunk
diff --git a/mm/vma.c b/mm/vma.c --- a/mm/vma.c +++ b/mm/vma.c
[ ... ]
quoted hunk ↗ jump to hunk
@@ -2800,10 +2855,16 @@ static int call_mmap_prepare(struct mmap_state *map, if (!desc->vm_ops) return -EINVAL; + /* Perform any preparatory tasks for mmap action. */ err = call_action_prepare(map, desc); if (err) return err; + /* Check the caller did nothing crazy. */ + err = mmap_prepare_validate(&prev_desc, desc); + if (err) + return err; + /* Update fields permitted to be changed. */ map->pgoff = desc->pgoff;
[Severity: High] This early return appears to have the same resource leak behavior as __compat_vma_mmap() above. Would a validation failure here also skip releasing driver resources or a newly assigned desc->vm_file before the VMA is fully established? -- Sashiko AI review · https://sashiko.dev/#/patchset/20260908-b4-mmap-prepare-vma-flag-sanify-v1-0-dacf19cce22b@kernel.org?part=3