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

[Lorenzo Stoakes (ARM)]

On Thu, Sep 24, 2026 at 09:19:26AM +0200, Vlastimil Babka (SUSE) wrote:
> On 9/23/26 19:45, 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.
> >
> > 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)
>
> Does it mean that patches are on the way to mainline that will break it, but
> it's unlikely they will be backported? Or there are no such patches yet?

Nope it's highly unlikely. You'd have to introduce a brand new mmap_prepare
etc. etc.

> Just curious... if it's the first case then with the amount of random stuff
> that goes to stable these days, I'd rather assume they could be backported
> at some point :)

Suren insisted on it being a fix and I didn't really want to argue.

I thought perhaps it hit something real but when writing the patch I asked
the LLM to actually check and it seems not, which is exactly what I thought
initially and why this wasn't a fix.

Anyway it's a small change so I think it's fine for stable.

>
> or out-of-tree modules which might be affected.
>
> That is never a concern, and even suggesting it can bring hch's wrath ;)

Yeah that's what I assumed.

>
> Anyway,
>
> Acked-by: Vlastimil Babka (SUSE) <vbabka@xxxxxxxxxx>

Thanks

>
>
> > ---
> >  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,
>

--
Cheers, Lorenzo