Re: [PATCH v3 04/40] mm: consistently validate VMA state after mmap[_prepare] hooks

On Wed, Sep 23, 2026 at 10:52:06PM -0400, Zi Yan wrote:

On Thu Sep 17, 2026 at 12:22 PM EDT, Lorenzo Stoakes (ARM) wrote:

quoted

When the f_op->mmap_prepare or deprecated f_op->mmap hooks are invoked, the
driver might have done something crazy that is not permitted by the kernel.

Currently we check for three such cases in __mmap_new_file_vma(), but only
if the legacy f_op->mmap hook is used:

* Did sparc ADI result in invalid flags?

* Did the driver alter vma->vm_start?

* Did the driver make a file-backed mapping on a read-only file writable?

Generalise these checks for both mmap_prepare and mmap and apply to all
invocations of mmap_file(), the f_op->mmap and f_op->mmap_prepare handling
in the core VMA code and the mmap_prepare compatibility layer.

Also extend the vm_start check to vm_end also - drivers must not change the
VMA range at all.

We also WARN_ON_ONCE() on these conditions as they are things that should
simply not occur in the kernel and it's important to call it out when it
does.

We invoke mmap_prepare_validate() after mmap_action_prepare(), as mmap
actions often manipulate state in the descriptor thus providing the final
state the VMA will be derived from.

Also call mmap_validate_vma_flags() in insert_vm_struct() to ensure that
special regions which are inserted (such as a VDSO or VVAR) also satisfy
the sanity checks.

This way every VMA established through an mmap hook, whether via mmap() or
the compatibility layer, or inserted via insert_vm_struct(), has been
validated. brk() VMAs never pass through a driver hook and so need no such
check.

While we're here, also fixup a couple disjoint blocks of #ifdef CONFIG_MMU.

Finally, update the VMA userland tests to reflect the change.

Signed-off-by: Lorenzo Stoakes (ARM) <ljs@kernel.org>
---
 mm/internal.h                   |  51 ++++++++++++--------
 mm/util.c                       |  19 ++++++--
 mm/vma.c                        | 100 ++++++++++++++++++++++++++++++++++------
 mm/vma.h                        |  25 ++++++++--
 tools/testing/vma/include/dup.h |  10 ++++
 5 files changed, 163 insertions(+), 42 deletions(-)

<snip>

quoted

+
+/* Check to ensure a driver hasn't done something crazy. */
+static int mmap_validate(unsigned long prev_start, unsigned long prev_end,
+			 unsigned long curr_start, unsigned long curr_end,
+			 const vma_flags_t *prev_flags,
+			 const vma_flags_t *curr_flags)
+{
+	bool was_maywrite, is_maywrite;
+
+	/* Drivers cannot alter the range of the VMA. */
+	if (WARN_ON_ONCE(prev_start != curr_start || prev_end != curr_end))
+		return -EINVAL;
+
+	was_maywrite = vma_flags_test(prev_flags, VMA_MAYWRITE_BIT);
+	is_maywrite = vma_flags_test(curr_flags, VMA_MAYWRITE_BIT);
+
+	/* A driver may not make a previously unwritable mapping writable. */
+	if (WARN_ON_ONCE(!was_maywrite && is_maywrite))

Is it driver specific or generally applicable to all mmap(_preppare)
operations? Is the comment too specific?

During my LLM quiz, making memfd write seals writable via a
hypothetically wrong shmem_mmap_prepare() implementation is an example
for this WARN_ON_ONCE. It is not driver related. Let me know if I get it
wrong.

Driver is taken to mean anything with an mmap or mmap_prepare hook, like a
general term for that.

If we start getting into calling it different if it's a file system or memfd or
something then it becomes quite hard to talk about it.

And yeah I hate that it's not a good name because driver makes you think
something in drivers/* or an OOT one or something but the kernel makes it vague
:)

Naming is hard...

Otherwise, LGTM.

Reviewed-by: Zi Yan <ziy@nvidia.com>

Thanks!

--
Best Regards,
Yan, Zi

--
Cheers, Lorenzo