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

[Samiullah Khawaja <skhawaja@google.com>]

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@amacapital.net> wrote:
>
>> On Sat, Oct 31, 2026 at 7:40 PM Andrew Morton <akpm@linux-foundation.org> wrote:
>> >
>> > On Sat, 31 Oct 2026 20:35:40 +0000 Samiullah Khawaja <skhawaja@google.com> 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