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

["Lorenzo Stoakes (ARM)" <ljs@kernel.org>]

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:
> > 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>
> > +
> > +/* 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