On Wed, Sep 23, 2026 at 09:47:20AM -0700, Suren Baghdasaryan wrote:
> On Thu, Sep 17, 2026 at 9:25 AM Lorenzo Stoakes (ARM) <[email protected]> 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) <[email protected]>
>
> Reviewed-by: Suren Baghdasaryan <[email protected]>
Thanks!
>
> > ---
> > 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(-)
> >
> > diff --git a/mm/internal.h b/mm/internal.h
> > index fe576d468af4..970fb34898b2 100644
> > --- a/mm/internal.h
> > +++ b/mm/internal.h
> > @@ -213,6 +213,24 @@ static inline void *folio_raw_mapping(const struct
> > folio *folio)
> > return (void *)(mapping & ~FOLIO_MAPPING_FLAGS);
> > }
> >
> > +/*
> > + * If the VMA has a close hook then close it, and since closing it might
> > leave
> > + * it in an inconsistent state which makes the use of any hooks suspect,
> > clear
> > + * them down by installing dummy empty hooks.
> > + */
> > +static inline void vma_close(struct vm_area_struct *vma)
> > +{
> > + if (vma->vm_ops && vma->vm_ops->close) {
> > + vma->vm_ops->close(vma);
> > +
> > + /*
> > + * The mapping is in an inconsistent state, and no further
> > hooks
> > + * may be invoked upon it.
> > + */
> > + vma->vm_ops = &vma_dummy_vm_ops;
> > + }
> > +}
> > +
> > /*
> > * This is a file-backed mapping, and is about to be memory mapped -
> > invoke its
> > * mmap hook and safely handle error conditions. On error, VMA hooks will
> > be
> > @@ -225,8 +243,12 @@ static inline void *folio_raw_mapping(const struct
> > folio *folio)
> > */
> > static inline int mmap_file(struct file *file, struct vm_area_struct *vma)
> > {
> > - int err = vfs_mmap(file, vma);
> > + const unsigned long prev_start = vma->vm_start;
> > + const unsigned long prev_end = vma->vm_end;
> > + const vma_flags_t prev_flags = vma->flags;
>
> nit: Might be just me but when I see prev_XXX in VMA-related code I
> picture previous VMA in the address space. Maybe call these orig_XXX?
Sure, will change.
>
> > + int err;
> >
> > + err = vfs_mmap(file, vma);
> > /*
> > * Either we tried to call the file hook for mmap() and an error
> > arose
> > * or a driver set vma->vm_ops = NULL intending there to be no VMA
> > @@ -239,26 +261,17 @@ 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;
> >
> > - return err;
> > -}
> > -
> > -/*
> > - * If the VMA has a close hook then close it, and since closing it might
> > leave
> > - * it in an inconsistent state which makes the use of any hooks suspect,
> > clear
> > - * them down by installing dummy empty hooks.
> > - */
> > -static inline void vma_close(struct vm_area_struct *vma)
> > -{
> > - if (vma->vm_ops && vma->vm_ops->close) {
> > - vma->vm_ops->close(vma);
> > -
> > - /*
> > - * The mapping is in an inconsistent state, and no further
> > hooks
> > - * may be invoked upon it.
> > - */
> > - vma->vm_ops = &vma_dummy_vm_ops;
> > + err = mmap_hook_validate(prev_start, prev_end, &prev_flags, vma);
> > + if (unlikely(err)) {
> > + vma->vm_start = prev_start;
> > + vma->vm_end = prev_end;
> > + vma_close(vma);
> > }
> > +
> > + return err;
> > }
> >
> > /* unmap_vmas is in mm/memory.c */
> > diff --git a/mm/util.c b/mm/util.c
> > index 016932780925..bdd5923eebc7 100644
> > --- a/mm/util.c
> > +++ b/mm/util.c
> > @@ -1224,19 +1224,28 @@ 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) {
> > - if (desc->vm_file != vma->vm_file)
> > - fput(desc->vm_file);
> > - return err;
> > - }
> > + if (err)
> > + goto err_put;
> > + /* Check the caller did nothing crazy. */
> > + err = mmap_prepare_validate(&prev_desc, desc);
> > + if (err)
> > + goto err_put;
> > /* Update the VMA from the descriptor. */
> > compat_set_vma_from_desc(vma, desc);
> > /* Complete any specified mmap actions. */
> > return mmap_action_complete(vma, &desc->action, /*is_compat=*/true);
> > +
> > +err_put:
> > + if (desc->vm_file != vma->vm_file)
> > + fput(desc->vm_file);
> > + return err;
> > }
> > EXPORT_SYMBOL(__compat_vma_mmap);
> >
> > diff --git a/mm/vma.c b/mm/vma.c
> > index 05d2c676672e..d6ed10cefc8f 100644
> > --- a/mm/vma.c
> > +++ b/mm/vma.c
> > @@ -2623,16 +2623,6 @@ static int __mmap_new_file_vma(struct mmap_state
> > *map,
> > return error;
> > }
> >
> > - /* Drivers cannot alter the address of the VMA. */
> > - WARN_ON_ONCE(map->addr != vma->vm_start);
> > - /*
> > - * Drivers should not permit writability when previously it was
> > - * disallowed.
> > - */
> > - VM_WARN_ON_ONCE(!vma_flags_same_pair(&map->vma_flags, &vma->flags)
> > &&
> > - !vma_flags_test(&map->vma_flags, VMA_MAYWRITE_BIT)
> > &&
> > - vma_test(vma, VMA_MAYWRITE_BIT));
> > -
> > map->vma_flags = vma->flags;
> >
> > return 0;
> > @@ -2710,11 +2700,6 @@ static int __mmap_new_vma(struct mmap_state *map,
> > struct vm_area_struct **vmap,
> > vma->flags = map->vma_flags;
> > }
> >
> > -#ifdef CONFIG_SPARC64
> > - /* TODO: Fix SPARC ADI! */
> > - WARN_ON_ONCE(!arch_validate_flags(map->vm_flags));
> > -#endif
> > -
> > /* Lock the VMA since it is modified after insertion into VMA tree
> > */
> > vma_start_write(vma);
> > vma_iter_store_new(vmi, vma);
> > @@ -2777,6 +2762,80 @@ static void __mmap_complete(struct mmap_state *map,
> > struct vm_area_struct *vma)
> > vma_set_page_prot(vma);
> > }
> >
> > +/* Check to ensure that the VMA flags of a newly mapped VMA are sane. */
> > +static int mmap_validate_vma_flags(const vma_flags_t *flags)
> > +{
> > +#ifdef CONFIG_SPARC64
> > + const vm_flags_t legacy_flags = vma_flags_to_legacy(*flags);
> > +
> > + /* TODO: Fix SPARC ADI! */
> > + if (WARN_ON_ONCE(!arch_validate_flags(legacy_flags)))
> > + return -EINVAL;
> > +#endif
> > +
> > + return 0;
> > +}
> > +
> > +/* 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))
> > + return -EINVAL;
> > +
> > + return mmap_validate_vma_flags(curr_flags);
> > +}
> > +
> > +/**
> > + * mmap_prepare_validate() - Ensure the driver hasn't violated invariants
> > in its
> > + * f_op->mmap_prepare hook.
> > + * @prev_desc: The VMA descriptor prior to the mmap_prepare hook being
> > called.
> > + * @desc: The VMA descriptor after the mmap_prepare hook has been called.
> > + *
> > + * Returns: 0 on success, otherwise an error.
> > + */
> > +int mmap_prepare_validate(const struct vm_area_desc *prev_desc,
> > + const struct vm_area_desc *desc)
> > +{
> > + return mmap_validate(prev_desc->start, prev_desc->end,
> > + desc->start, desc->end,
> > + &prev_desc->vma_flags, &desc->vma_flags);
> > +}
> > +
> > +/**
> > + * mmap_hook_validate() - Ensure the driver hasn't violated invariants in
> > + * its f_op->mmap hook.
> > + * @prev_start: The start of the mapping prior to the mmap hook.
> > + * @prev_end: The end of the mapping prior to the mmap hook.
> > + * @prev_flags: The VMA flags set for the VMA prior to the mmap hook.
> > + * @vma: The VMA after the hook has been applied.
> > + *
> > + * Returns: 0 on success, otherwise an error.
> > + */
> > +int mmap_hook_validate(unsigned long prev_start, unsigned long prev_end,
> > + const vma_flags_t *prev_flags,
> > + const struct vm_area_struct *vma)
> > +{
> > + const unsigned long start = vma->vm_start;
> > + const unsigned long end = vma->vm_end;
> > + const vma_flags_t *flags = &vma->flags;
> > +
> > + return mmap_validate(prev_start, prev_end, start, end, prev_flags,
> > + flags);
> > +}
> > +
> > static int call_action_prepare(struct mmap_state *map,
> > struct vm_area_desc *desc)
> > {
> > @@ -2803,6 +2862,7 @@ static int call_action_prepare(struct mmap_state *map,
> > static int call_mmap_prepare(struct mmap_state *map,
> > struct vm_area_desc *desc)
> > {
> > + const struct vm_area_desc prev_desc = *desc;
> > int err;
> >
> > /* Invoke the hook. */
> > @@ -2822,6 +2882,11 @@ static int call_mmap_prepare(struct mmap_state *map,
> > 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;
> > map->vma_flags = desc->vma_flags;
> > @@ -3457,10 +3522,15 @@ int __vm_munmap(unsigned long start, size_t len,
> > bool unlock)
> > int insert_vm_struct(struct mm_struct *mm, struct vm_area_struct *vma)
> > {
> > unsigned long charged = vma_pages(vma);
> > + int err;
> >
> > if (find_vma_intersection(mm, vma->vm_start, vma->vm_end))
> > return -ENOMEM;
> >
> > + err = mmap_validate_vma_flags(&vma->flags);
> > + if (err)
> > + return err;
> > +
> > if (vma_test(vma, VMA_ACCOUNT_BIT) &&
> > security_vm_enough_memory_mm(mm, charged))
> > return -ENOMEM;
> > diff --git a/mm/vma.h b/mm/vma.h
> > index f15faa83f3d6..b2c3bc832a48 100644
> > --- a/mm/vma.h
> > +++ b/mm/vma.h
> > @@ -782,14 +782,19 @@ struct vm_area_struct *vm_area_alloc(struct mm_struct
> > *mm);
> > struct vm_area_struct *vm_area_dup(struct vm_area_struct *orig);
> > void vm_area_free(struct vm_area_struct *vma);
> >
> > -/* vma_exec.c */
> > #ifdef CONFIG_MMU
> > +int mmap_prepare_validate(const struct vm_area_desc *prev_desc,
> > + const struct vm_area_desc *desc);
> > +
> > +int mmap_hook_validate(unsigned long prev_start, unsigned long prev_end,
> > + const vma_flags_t *prev_flags,
> > + const struct vm_area_struct *vma);
> > +
> > +/* vma_exec.c */
> > int create_init_stack_vma(struct mm_struct *mm, struct vm_area_struct
> > **vmap,
> > unsigned long *top_mem_p);
> > int relocate_vma_down(struct vm_area_struct *vma, unsigned long shift);
> > -#endif
> >
> > -#ifdef CONFIG_MMU
> > /*
> > * Denies creating a writable executable mapping or gaining executable
> > permissions.
> > *
> > @@ -838,6 +843,20 @@ static inline bool map_deny_write_exec(const
> > vma_flags_t *old,
> >
> > return false;
> > }
> > +#else
> > +static inline int mmap_prepare_validate(const struct vm_area_desc
> > *prev_desc,
> > + const struct vm_area_desc *desc)
> > +{
> > + return 0;
> > +}
> > +
> > +static inline int mmap_hook_validate(unsigned long prev_start,
> > + unsigned long prev_end,
> > + const vma_flags_t *prev_flags,
> > + const struct vm_area_struct *vma)
> > +{
> > + return 0;
> > +}
> > #endif
> >
> > struct vm_area_struct *__install_special_mapping(struct mm_struct *mm,
> > diff --git a/tools/testing/vma/include/dup.h
> > b/tools/testing/vma/include/dup.h
> > index 2fd422789717..2986ae6ca1e5 100644
> > --- a/tools/testing/vma/include/dup.h
> > +++ b/tools/testing/vma/include/dup.h
> > @@ -1359,13 +1359,23 @@ static inline int vfs_mmap_prepare(struct file
> > *file, struct vm_area_desc *desc)
> > return file->f_op->mmap_prepare(desc);
> > }
> >
> > +int mmap_prepare_validate(const struct vm_area_desc *prev_desc,
> > + const struct vm_area_desc *desc);
> > +
> > static inline 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;
> > /* Update the VMA from the descriptor. */
> >
> > --
> > 2.55.0
> >
--
Cheers, Lorenzo
