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

["Lorenzo Stoakes (ARM)" <ljs@kernel.org>]

On Thu, Sep 24, 2026 at 09:57:03AM +0100, Pedro Falcato wrote:
> 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.
>
> Hmm, can you explain how this is safe? I was wondering if _any_ kind of
> mismatch should be a WARN_ON (sounds like something odd is happening if
> they don't match). But if the new state is practically discardable, that
> doesn't make sense.

I mean this is asking me to defend how the kernel has worked forever :) don't
make me responsible for that...

For the VAST majority of cases, vm_ops is static. So they WILL be identical.

shmem does something really stupid with that which it shouldn't do (vary the
vm_ops on whether the file is unlinked or not), and maybe that needs a follow
up.

But anyway, one thing that saves us from a lot of this crap is that the majority
of things that store meaningful state there are non-mergeable,
i.e. VMA_PFNMAP_BIT, et al.

Of those that remain, the state is very often simply forwarding file state,
like:

	vma->vm_private_data = file->private_data;

At which point you already have equality.

Even if shared file state things might be tied to VMAs, but that should always
come out in the wash.

If you, for instance, stored the number of mappings there as a reference count (I
think fairly typical) then you automatically get the correct number because
vm_ops->close() will be called on a fully merged VMA (but not a partially merged
one which is correct).

Typical usage for legacy hook is mmap -> set refcount 1, open refcount++
(e.g. on split), close refcount--.

The mmap_prepare equivalent for that is vm_ops->mapped set refcount 1 (you
mustn't set state like that in mmap_prepare), and open/close as usual.

Another thing here is the idea that vm_private_data is ephemeral state _tied to
the VMA_.

It is already the case that that state will not be updated on expand/shrink of a
VMA, and so it is the case that if the VMA is unmapped for any reason the state
disappears.

So this ephemeral quality extends, by convention, to not impacting merge.

Anyway, I sent that 40 patch series (sorry for the size) in large part to
_strengthen_ assumptions about mmap_prepare (and, via legacy, mmap) hook
behaviour.

In general part of the mmap_prepare project is to allow us to get rid of
vagueries of the past and be able to make strong assumptions about behaviour.

So in the same vein, I'll do an audit of the codebase to make sure nothing
relies on this.

It might be interesting as well to audit what this data is actually set
to.

Maybe it makes sense to enforce that vm_private_data is always set to file
private dataa?

But that then implies that the VMA private data is unnecessary.

I do think requiring equivalent vm_ops might be a good thing to change, and
then to at least document the fact that vm_private_data is ephemeral like
this on merge.

Let me investigate how they're used and I can figure out how to improve
things.

>
> --
> Pedro

--
Cheers, Lorenzo