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]> > --- > 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? > + 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 >
