Re: [PATCH] mm/vma: predicate setting mmap_prepare VMA fields on new vma alloc

[Lorenzo Stoakes (ARM)]

On Wed, Sep 23, 2026 at 06:45:41PM +0100, Lorenzo Stoakes (ARM) wrote:
> It only makes sense to manipulate VMA fields if a new VMA was allocated,
> rather than merged.
>
> VMA merging does not compare vm_ops or vm_private_data, so a merged VMA
> keeps its own, which is also what the legacy f_op->mmap path does since it
> never touches an existing VMA.
>
> Currently, these fields will get overwritten by whatever state is
> established in the mmap_prepare hook, and if the VMA was merged,
> vm_ops->mapped will not have been called, so this could destructively clear
> existing state without replacing it with anything valid.
>
> There is an implicit requirement that vm_private_data and vm_ops are
> fungible across VMAs which means that losing the 'new' state is
> fine.
>
> However in this case the 'old' state is being overwritten by potentially
> invalid 'new' state, so this must be rectified.
>
> Additionally constify have_mmap_prepare while here.
>
> All existing in-tree users either derive state for the tree or are
> unmergeable due to VMA flags, so this has no direct impact.

Instantly noticed a typo when I hit send (ugh).

'derive state for the tree' should be 'derive state from the file'. It's late :)

>
> Fixes: c84bf6dd2b83 ("mm: introduce new .mmap_prepare() file callback")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Lorenzo Stoakes (ARM) <ljs@xxxxxxxxxx>
> ---
> Note that this is cc: stable to account for any possible back-ports that could
> break it (unlikely) or out-of-tree modules which might be affected.
> ---
>  mm/vma.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/mm/vma.c b/mm/vma.c
> index 9f0a0acf694a..6cde67883fb0 100644
> --- a/mm/vma.c
> +++ b/mm/vma.c
> @@ -2849,7 +2849,7 @@ static unsigned long __mmap_region(struct file *file, unsigned long addr,
>  {
>  	struct mm_struct *mm = current->mm;
>  	struct vm_area_struct *vma = NULL;
> -	bool have_mmap_prepare = file && file->f_op->mmap_prepare;
> +	const bool have_mmap_prepare = file && file->f_op->mmap_prepare;
>  	VMA_ITERATOR(vmi, mm, addr);
>  	const pgoff_t anon_pgoff = addr >> PAGE_SHIFT;
>  	MMAP_STATE(map, mm, &vmi, addr, len, pgoff, anon_pgoff, vma_flags, file);
> @@ -2892,7 +2892,7 @@ static unsigned long __mmap_region(struct file *file, unsigned long addr,
>  		allocated_new = true;
>  	}
>
> -	if (have_mmap_prepare)
> +	if (have_mmap_prepare && allocated_new)
>  		set_vma_user_defined_fields(vma, &map);
>
>  	__mmap_complete(&map, vma);
>
> ---
> base-commit: fe2ec83746e501645709761605c2464a44fd2929
> change-id: 20260923-fix-mmap-prepare-overwrite-6304d112a4c7
>
> Best regards,
> --
> Lorenzo Stoakes (ARM) <ljs@xxxxxxxxxx>
>

--
Cheers, Lorenzo