Re: [PATCH] mm/gup: check folio->mapping under folio lock in memfd_pin_folios()

[Samiullah Khawaja]

On Sat, Oct 31, 2026 at 09:25:07PM -0700, Andrew Morton wrote:
> On Sat, 31 Oct 2026 20:20:03 -0700 Andy Lutomirski <luto@xxxxxxxxxxxxxx> wrote:
>
> > On Sat, Oct 31, 2026 at 7:40 PM Andrew Morton <akpm@xxxxxxxxxxxxxxxxxxxx> wrote:
> > >
> > > On Sat, 31 Oct 2026 20:35:40 +0000 Samiullah Khawaja <skhawaja@xxxxxxxxxx> wrote:
> > >
> > > > memfd_pin_folios() pins folios from the page cache without taking the
> > > > folio lock. So khugepaged or MADV_COLLAPSE can replace the folio after
> > > > its refcount check while memfd_pin_folios() still pins the old folio,
> > > > and the caller ends up with a folio that is no longer part of the memfd.
> > > >
> > > > Lock the folio and recheck folio->mapping before pinning it, like other
> > > > page cache users do. If the folio was removed, retry the folio lookup
> > > > from the index that was removed.
> > >
> > > Can we please include a clear description of the userspace-visible
> > > runtime effects of the bug?
> >
> > It's straight-up data corruption if you manage to trigger it, no?  If
> > you manage to race a memfd_pin_folios() against collapse,
> > memfd_pin_folios() will pin the wrong folio.
> >
> > Fortunately, there are a whopping three callers in the whole tree.
> > You can hit it from udmabuf or iommufd or memfd LUO.
> >
> > The former two will direct DMA to the wrong folio.  Fortunately it's
> > not UAF as the DMA will be directed to a folio that is still pinned,
> > but it's the wrong one.  DMA reads will read potentially stale data
> > and DMA writes will overwrite that stale data but not update the live
> > copy.  (The DMA will not be directed to some other application's
> > memory, just to a leftover copy of the calling application's memory.)
> >
> > As far as I know, the effect with LUO would be for stale data to
> > reappear in the memfd after handover.  With the patchset that started
> > this whole discussion applied, I think it might result in misdirected
> > DMA to freed memory (I do not stand by that claim at all), but that
> > patchset is not applied.
>
> Thanks.  How's this for a changelog?
>
> : memfd_pin_folios() pins folios from the page cache without taking the
> : folio lock. khugepaged or MADV_COLLAPSE can therefore replace a folio
> : after checking its refcount while memfd_pin_folios() still pins the old
> : folio. The caller then receives a folio which is no longer part of the
> : memfd.
> :
> : For udmabuf and iommufd, this can cause data corruption: DMA accesses
> : the detached folio rather than the live memfd contents. DMA reads can
> : return stale data, while DMA writes update the detached copy and are
> : lost from the live memfd. The detached folio remains pinned, so these
> : accesses do not constitute a use-after-free or target another
> : application's memory. For memfd live update, the stale contents can
> : reappear in the memfd after handover.
> :
> : Lock the folio and recheck folio->mapping before pinning it, like other
> : page cache users do. If the folio was removed, retry the folio lookup
> : from the index that was removed.

This explanation sounds good to me.

Please let me know if you want me to send a v2 with this changelog.

Thanks,
Sami