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) <[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(-)
>
<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.

Otherwise, LGTM.

Reviewed-by: Zi Yan <[email protected]>


-- 
Best Regards,
Yan, Zi


Reply via email to