Re: [PATCH v2 2/3] userfaultfd: refuse to __mfill_atomic_pte() for unsupported VMAs

[David CARLIER]

On Thu, 28 May 2026 at 08:33, Mike Rapoport <rppt@xxxxxxxxxx> wrote:
>
> On Wed, May 27, 2026 at 08:09:01PM +0100, David CARLIER wrote:
> > Hi,
> >
> > just chiming it
> >
> > On Wed, 27 May 2026 at 19:48, Mike Rapoport <rppt@xxxxxxxxxx> wrote:
> > >
> > > From: "Mike Rapoport (Microsoft)" <rppt@xxxxxxxxxx>
> > >
> > > __mfill_atomic_pte() unconditionally dereferences ops because there is an
> > > assumption that VMAs that can undergo mfill_* operations are vetted on
> > > registration and must have valid vm_uffd_ops.
> > >
> > > Add a guard against potential bugs and make sure __mfill_atomic_pte() bails
> > > out if ops is NULL.
> > >
> > > Suggested-by: Lorenzo Stoakes <ljs@xxxxxxxxxx>
> > > Fixes: ad9ac3081332 ("userfaultfd: introduce vm_uffd_ops->alloc_folio()")
> > > Signed-off-by: Mike Rapoport (Microsoft) <rppt@xxxxxxxxxx>
> > > ---
> > >  mm/userfaultfd.c | 5 +++++
> > >  1 file changed, 5 insertions(+)
> > >
> > > diff --git a/mm/userfaultfd.c b/mm/userfaultfd.c
> > > index e5d2fb3ce2c1..2872c71bbf36 100644
> > > --- a/mm/userfaultfd.c
> > > +++ b/mm/userfaultfd.c
> > > @@ -552,6 +552,11 @@ static int __mfill_atomic_pte(struct mfill_state *state,
> > >         struct folio *folio;
> > >         int ret;
> > >
> > > +       if (!ops) {
> > > +               VM_WARN_ONCE(1, "UFFDIO_COPY for unsupported VMA");
> >
> > nit: Is the warning message a bit too specific ? i.e. Also handle
> > MFILL_ATOMIC_ZEROPAGE case.
>
> It handles ZEROPAGE case by actually copying a zeroed page there so in a
> sense it's still a copy.
>
> Differentiating COPY and ZEROPAGE here will add complexity that is not
> justified for a message that should be never printed.
>
> > > +               return -EOPNOTSUPP;
> > > +       }
> > > +
> > >         folio = ops->alloc_folio(state->vma, state->dst_addr);
> > >         if (!folio)
> > >                 return -ENOMEM;
> > > --
> > > 2.53.0
> > >
> >
> > Cheers.
> >
>
> --
> Sincerely yours,
> Mike.

ACK.

Reviewed-by: David CARLIER <devnexen@xxxxxxxxx>