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

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

[Samiullah Khawaja]

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.

Fixes: 89c1905d9c14 ("mm/gup: introduce memfd_pin_folios() for pinning memfd folios")
Reported-by: Andy Lutomirski <luto@amacapital.net>
Closes: https://lore.kernel.org/all/CALCETrV5eqg6d9=+BQhCpRwBNe3aCQiut8vxPBs8b7PToqjaHg@mail.gmail.com/
Cc: stable@vger.kernel.org
Signed-off-by: Samiullah Khawaja <skhawaja@google.com>
---
 mm/gup.c | 29 +++++++++++++++++++++--------
 1 file changed, 21 insertions(+), 8 deletions(-)

diff --git a/mm/gup.c b/mm/gup.c
index ba4907b35232..2b405cce2ca5 100644
--- a/mm/gup.c
+++ b/mm/gup.c
@@ -3471,7 +3471,7 @@ long memfd_pin_folios(struct file *memfd, loff_t start, loff_t end,
 {
 	unsigned int flags, nr_folios, nr_found;
 	unsigned int i, pgshift = PAGE_SHIFT;
-	pgoff_t start_idx, end_idx;
+	pgoff_t start_idx, end_idx, retry_idx;
 	struct folio *folio = NULL;
 	struct folio_batch fbatch;
 	struct hstate *h;
@@ -3512,6 +3512,7 @@ long memfd_pin_folios(struct file *memfd, loff_t start, loff_t end,
 			 * reason, we try to allocate them and add them to the
 			 * page cache.
 			 */
+			retry_idx = start_idx;
 			nr_found = filemap_get_folios_contig(memfd->f_mapping,
 							     &start_idx,
 							     end_idx,
@@ -3524,7 +3525,21 @@ long memfd_pin_folios(struct file *memfd, loff_t start, loff_t end,
 			for (i = 0; i < nr_found; i++) {
 				folio = fbatch.folios[i];
 
+				/*
+				 * The folio might have been removed from the
+				 * page cache (e.g., truncated or collapsed)
+				 * after the lookup. Recheck under the folio
+				 * lock.
+				 */
+				folio_lock(folio);
+				if (unlikely(folio->mapping != memfd->f_mapping)) {
+					folio_unlock(folio);
+					start_idx = retry_idx;
+					break;
+				}
+
 				if (try_grab_folio(folio, 1, FOLL_PIN)) {
+					folio_unlock(folio);
 					folio_batch_release(&fbatch);
 					ret = -EINVAL;
 					goto err;
@@ -3535,14 +3550,12 @@ long memfd_pin_folios(struct file *memfd, loff_t start, loff_t end,
 				 * page cache that were never cleared.
 				 */
 				if (!folio_test_uptodate(folio)) {
-					folio_lock(folio);
-					if (!folio_test_uptodate(folio)) {
-						folio_zero_range(folio, 0, folio_size(folio));
-						flush_dcache_folio(folio);
-						folio_mark_uptodate(folio);
-					}
-					folio_unlock(folio);
+					folio_zero_range(folio, 0, folio_size(folio));
+					flush_dcache_folio(folio);
+					folio_mark_uptodate(folio);
 				}
+				retry_idx = folio_next_index(folio);
+				folio_unlock(folio);
 
 				if (nr_folios == 0)
 					*offset = offset_in_folio(folio, start);

base-commit: 96995467b456942f5bf1c7f2c5c004467f9dd14e
-- 
2.56.0.317.g454f8302e1-goog

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

[Andy Lutomirski]

On Sat, Oct 31, 2026 at 1:35 PM 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.
>
> Fixes: 89c1905d9c14 ("mm/gup: introduce memfd_pin_folios() for pinning memfd folios")
> Reported-by: Andy Lutomirski <luto@amacapital.net>

Reviewed-by: Andy Lutomirski <luto@amacapital.net>

(with the caveat that I still don't like the fact that pinning a folio
does not keep it associated with the mapping.)

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

[Andrew Morton]

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?

> Fixes: 89c1905d9c14 ("mm/gup: introduce memfd_pin_folios() for pinning memfd folios")
> Reported-by: Andy Lutomirski <luto@amacapital.net>
> Closes: https://lore.kernel.org/all/CALCETrV5eqg6d9=+BQhCpRwBNe3aCQiut8vxPBs8b7PToqjaHg@mail.gmail.com/

Includes the words

> The issue is live update doesn't preserve those background punched
> folios and now we have created an UAF attack on the kernel across
> kexec if you don't do something about it.

Is that the extent of it, or are there other effects?

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

[Andy Lutomirski]

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.

>
> > Fixes: 89c1905d9c14 ("mm/gup: introduce memfd_pin_folios() for pinning memfd folios")
> > Reported-by: Andy Lutomirski <luto@amacapital.net>
> > Closes: https://lore.kernel.org/all/CALCETrV5eqg6d9=+BQhCpRwBNe3aCQiut8vxPBs8b7PToqjaHg@mail.gmail.com/
>
> Includes the words
>
> > The issue is live update doesn't preserve those background punched
> > folios and now we have created an UAF attack on the kernel across
> > kexec if you don't do something about it.
>
> Is that the extent of it, or are there other effects?

I *think* that the UAF can't actually happen in any upstream kernel.

-- 
Andy Lutomirski
AMA Capital Management, LLC

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

[Andrew Morton]

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.

Samiullah's patch assumes the presence of Chris Mason's "mm/gup: fix
clearing folios in memfd_pin_folios()" which is in mm-next, so... 
->David

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@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