On Fri, Sep 25, 2026 at 12:28:49AM -0700, Suren Baghdasaryan wrote:
> On Thu, Sep 17, 2026 at 9:25 AM Lorenzo Stoakes (ARM) <[email protected]> wrote:
> >
> > When a user requests an mmap_action be performed in mmap_prepare, this
> > involves populating the VMA range with data.
> >
> > However, if the VMA is mergeable, it might then mistakenly be merged with
> > another VMA without having populated the range.
> >
> > Every mmap action currently available sets VMA flags such that the VMA
> > cannot be merged.
> >
> > However, to ensure that no future mmap action falls foul of this, assert
> > that this is the case upon mmap_prepare validation.
> >
> > Signed-off-by: Lorenzo Stoakes (ARM) <[email protected]>
>
> Reviewed-by: Suren Baghdasaryan <[email protected]>

Thanks!

>
> > ---
> >  mm/vma.c | 9 +++++++++
> >  1 file changed, 9 insertions(+)
> >
> > diff --git a/mm/vma.c b/mm/vma.c
> > index d6ed10cefc8f..62f2ce1ad5a1 100644
> > --- a/mm/vma.c
> > +++ b/mm/vma.c
> > @@ -2809,6 +2809,15 @@ static int mmap_validate(unsigned long prev_start, 
> > unsigned long prev_end,
> >  int mmap_prepare_validate(const struct vm_area_desc *prev_desc,
> >                           const struct vm_area_desc *desc)
> >  {
> > +       /*
> > +        * It is not valid to execute mmap actions for VMAs which can be 
> > merged,
> > +        * as any such merge would leave portions of the mapping incorrectly
> > +        * unmapped.
> > +        */
> > +       if (vma_flags_can_merge(&desc->vma_flags) &&
> > +           WARN_ON_ONCE(desc->action.type != MMAP_NOTHING))
>
> Any reason you chose this "if (A && WARN_ON_ONCE(B))" pattern instead
> of a simpler "if (WARN_ON_ONCE(A && B))"? Unless there are races
> between A and B updates, I think these would be equivalent, right?

They are equivalent yes!

So it's more of a style thing, as that reads less clearly to me:

        if (WARN_ON_ONCE(vma_flags_can_merge(&desc->vma_flags) &&
                         desc->action.type != MMAP_NOTHING))

And this version highlights what is the broken thing here (action type set) a
little more clearly though obviously that's only if mergeable so arguable.

I don't have a really strong opinion though I can change it if you prefer?

>
> > +               return -EINVAL;
> > +
> >         return mmap_validate(prev_desc->start, prev_desc->end,
> >                              desc->start, desc->end,
> >                              &prev_desc->vma_flags, &desc->vma_flags);
> >
> > --
> > 2.55.0
> >

--
Cheers, Lorenzo

Reply via email to