[PATCH v2 0/2] mm/memory: refuse to map page cache folios that are not uptodate

[PATCH v2 0/2] mm/memory: refuse to map page cache folios that are not uptodate

[Kiryl Shutsemau (Meta)]

A page cache folio that is not uptodate holds data that was never read
in or cleared. Each path that maps page cache checks for this itself,
and a path that misses the check leaks stale memory to userspace.
MADV_COLLAPSE did exactly that with fallocated shmem THPs.

Patch 2 adds a backstop: warn and refuse the mapping in __do_fault()
and do_set_pmd().

Patch 1 fixes a race the backstop would trip over. do_shared_fault()
unlocks the folio around ->page_mkwrite(), and a writeback error can
clear PG_uptodate in that window. The fault goes on to map the folio,
and stores through the mapping can be lost when it is read in again.
Retry the fault instead.

v2:
 - New patch 1: recheck PG_uptodate after ->page_mkwrite() (Sashiko)
 - Check in __do_fault(), before the folio is unlocked, instead of in
   finish_fault()
 - WARN_ON_ONCE() without a message
 - Rebase on current mm-new

v1: https://lore.kernel.org/all/20261106144323.3634477-1-kirill@shutemov.name/

Kiryl Shutsemau (Meta) (2):
  mm/memory: recheck PG_uptodate after ->page_mkwrite()
  mm/memory: refuse to map page cache folios that are not uptodate

 mm/memory.c | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

Range-diff against v1:
-:  ------------ > 1:  632ab50e643d mm/memory: recheck PG_uptodate after ->page_mkwrite()
1:  af1dd44c0e1b ! 2:  1e329f330c41 mm/memory: refuse to map page cache folios that are not uptodate
    @@ Commit message
         leaks stale memory silently. MADV_COLLAPSE did exactly that with
         fallocated shmem THPs until recently.
     
    -    Warn and refuse the mapping in finish_fault() and do_set_pmd().
    -    finish_fault() checks the folio ->fault() returned rather than the one
    -    it maps: for CoW, do_cow_fault() copies the page cache folio and maps
    -    the copy. do_set_pmd() covers MADV_COLLAPSE, which maps without a
    -    fault.
    +    Warn and refuse the mapping in __do_fault() and do_set_pmd().
    +
    +    __do_fault() checks the folio ->fault() returned while it is still
    +    locked. do_shared_fault() unlocks it for ->page_mkwrite(), and a
    +    writeback error can clear PG_uptodate after that; that case retries the
    +    fault instead. For CoW, __do_fault() sees the page cache folio that
    +    do_cow_fault() copies from. do_set_pmd() covers MADV_COLLAPSE, which
    +    maps without a fault.
     
         The fault gets SIGBUS, as for a folio that fails to read in.
         MADV_COLLAPSE fails and leaves the range to be faulted in.
    @@ Commit message
     
      ## mm/memory.c ##
     @@ mm/memory.c: static vm_fault_t __do_fault(struct vm_fault *vmf)
    + 	else
    + 		VM_BUG_ON_PAGE(!folio_test_locked(folio), vmf->page);
    + 
    ++	if (WARN_ON_ONCE(file_folio_not_uptodate(folio))) {
    ++		folio_unlock(folio);
    ++		folio_put(folio);
    ++		vmf->page = NULL;
    ++		return VM_FAULT_SIGBUS;
    ++	}
    ++
      	return ret;
      }
      
    -+/*
    -+ * A page cache folio that is not uptodate holds data that was never read in
    -+ * or cleared, and mapping it leaks stale memory to userspace. The callers are
    -+ * expected to check, so treat a hit as a kernel bug and refuse the mapping.
    -+ */
    -+static bool file_folio_not_uptodate(struct folio *folio)
    -+{
    -+	if (likely(folio_test_uptodate(folio)))
    -+		return false;
    -+	/* Driver pages from ->fault() never get PG_uptodate */
    -+	if (folio_test_anon(folio) || !folio->mapping)
    -+		return false;
    -+	WARN_ONCE(1, "refusing to map a page cache folio that is not uptodate\n");
    -+	return true;
    -+}
    -+
    - #ifdef CONFIG_TRANSPARENT_HUGEPAGE
    - static void deposit_prealloc_pte(struct vm_fault *vmf)
    - {
     @@ mm/memory.c: vm_fault_t do_set_pmd(struct vm_fault *vmf, struct folio *folio, struct page *pa
      		return ret;
      	page = &folio->page;
      
    -+	if (file_folio_not_uptodate(folio))
    ++	if (WARN_ON_ONCE(file_folio_not_uptodate(folio)))
     +		return VM_FAULT_SIGBUS;
     +
      	/*
      	 * Just backoff if any subpage of a THP is corrupted otherwise
      	 * the corrupted page may mapped by PMD silently to escape the
    -@@ mm/memory.c: vm_fault_t finish_fault(struct vm_fault *vmf)
    - 	unsigned long addr;
    - 	bool needs_fallback = false;
    - 
    -+	/*
    -+	 * Check the folio ->fault() returned. For CoW that is the source of the
    -+	 * copy; DAX CoW has none.
    -+	 */
    -+	if (vmf->page) {
    -+		folio = page_folio(vmf->page);
    -+		if (file_folio_not_uptodate(folio))
    -+			return VM_FAULT_SIGBUS;
    -+	}
    -+
    - fallback:
    - 	addr = vmf->address;
    - 

base-commit: b95f8bb86713c860ce8536d22a988b949b884d5f
-- 
2.54.0

[PATCH v2 1/2] mm/memory: recheck PG_uptodate after ->page_mkwrite()

[Kiryl Shutsemau (Meta)]

do_shared_fault() unlocks the folio around ->page_mkwrite(). Writeback
can lock it in that window, fail, and clear PG_uptodate with the folio
left in the page cache. ecryptfs_writepages() does this when encryption
fails. filemap_page_mkwrite() relocks the folio and checks only
->mapping, so the fault maps a folio that is not uptodate:

    CPU0                            CPU1
    do_shared_fault()
      folio_unlock()
                                    ecryptfs_writepages()
                                      folio_clear_uptodate()
      filemap_page_mkwrite()
        folio_lock()
      finish_fault()

The next read(2) or fault on the folio reads it in again from the file,
over anything stored through the mapping in the meantime. Those stores
are lost.

Recheck PG_uptodate once ->page_mkwrite() returns with the folio locked,
and retry the fault if it is clear, the same way filemap_fault() retries
on a cached folio that lost PG_uptodate. Skip folios outside the page
cache: driver pages from ->fault() never get PG_uptodate and would
retry forever.

The check stays out of do_page_mkwrite(): wp_page_shared() calls it on
a folio that is still mapped, and a retry there would fault on the same
PTE forever.

Reported-by: Sashiko <sashiko-bot@kernel.org>
Closes: https://lore.kernel.org/sashiko-outbox-202966@kernel.org/
Signed-off-by: Kiryl Shutsemau (Meta) <kas@kernel.org>
---
 mm/memory.c | 22 ++++++++++++++++++++++
 1 file changed, 22 insertions(+)

diff --git a/mm/memory.c b/mm/memory.c
index fe72e66c9c2e..cc1d0b7d15c7 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -5613,6 +5613,18 @@ static vm_fault_t do_anonymous_page(struct vm_fault *vmf)
 	return VM_FAULT_OOM;
 }
 
+/*
+ * A page cache folio that is not uptodate holds data that was never read in
+ * or cleared, and mapping it leaks stale memory to userspace.
+ */
+static bool file_folio_not_uptodate(struct folio *folio)
+{
+	if (likely(folio_test_uptodate(folio)))
+		return false;
+	/* Driver pages from ->fault() never get PG_uptodate */
+	return !folio_test_anon(folio) && folio->mapping;
+}
+
 /*
  * Either the VMA lock or the mmap_lock must have been held on entry
  * (see FAULT_FLAG_VMA_LOCK) and may have been released depending on
@@ -6163,6 +6175,16 @@ static vm_fault_t do_shared_fault(struct vm_fault *vmf)
 			folio_put(folio);
 			return tmp;
 		}
+
+		/*
+		 * The folio was unlocked for ->page_mkwrite(). If it lost
+		 * PG_uptodate meanwhile, retry the fault to read it in again.
+		 */
+		if (unlikely(file_folio_not_uptodate(folio))) {
+			folio_unlock(folio);
+			folio_put(folio);
+			return VM_FAULT_NOPAGE;
+		}
 	}
 
 	ret |= finish_fault(vmf);
-- 
2.54.0

Re: [PATCH v2 1/2] mm/memory: recheck PG_uptodate after ->page_mkwrite()

[Pedro Falcato]

On Mon, Nov 09, 2026 at 12:04:01PM +0000, Kiryl Shutsemau (Meta) wrote:
> do_shared_fault() unlocks the folio around ->page_mkwrite(). Writeback
> can lock it in that window, fail, and clear PG_uptodate with the folio
> left in the page cache. ecryptfs_writepages() does this when encryption
> fails. filemap_page_mkwrite() relocks the folio and checks only
> ->mapping, so the fault maps a folio that is not uptodate:
> 
>     CPU0                            CPU1
>     do_shared_fault()
>       folio_unlock()
>                                     ecryptfs_writepages()
>                                       folio_clear_uptodate()
>       filemap_page_mkwrite()
>         folio_lock()
>       finish_fault()
> 
> The next read(2) or fault on the folio reads it in again from the file,
> over anything stored through the mapping in the meantime. Those stores
> are lost.
> 
> Recheck PG_uptodate once ->page_mkwrite() returns with the folio locked,
> and retry the fault if it is clear, the same way filemap_fault() retries
> on a cached folio that lost PG_uptodate. Skip folios outside the page
> cache: driver pages from ->fault() never get PG_uptodate and would
> retry forever.
> 
> The check stays out of do_page_mkwrite(): wp_page_shared() calls it on
> a folio that is still mapped, and a retry there would fault on the same
> PTE forever.
> 
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Closes: https://lore.kernel.org/sashiko-outbox-202966@kernel.org/
> Signed-off-by: Kiryl Shutsemau (Meta) <kas@kernel.org>
> ---
>  mm/memory.c | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
> 
> diff --git a/mm/memory.c b/mm/memory.c
> index fe72e66c9c2e..cc1d0b7d15c7 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -5613,6 +5613,18 @@ static vm_fault_t do_anonymous_page(struct vm_fault *vmf)
>  	return VM_FAULT_OOM;
>  }
>  
> +/*
> + * A page cache folio that is not uptodate holds data that was never read in
> + * or cleared, and mapping it leaks stale memory to userspace.
> + */
> +static bool file_folio_not_uptodate(struct folio *folio)
> +{
> +	if (likely(folio_test_uptodate(folio)))
> +		return false;
> +	/* Driver pages from ->fault() never get PG_uptodate */
> +	return !folio_test_anon(folio) && folio->mapping;

FYI you can wrap this line in an unlikely() and it has the same effect as
wrapping every call:
	return unlikely(!folio_test_anon(folio) && folio->mapping);

> +}
> +
>  /*
>   * Either the VMA lock or the mmap_lock must have been held on entry
>   * (see FAULT_FLAG_VMA_LOCK) and may have been released depending on
> @@ -6163,6 +6175,16 @@ static vm_fault_t do_shared_fault(struct vm_fault *vmf)
>  			folio_put(folio);
>  			return tmp;
>  		}
> +
> +		/*
> +		 * The folio was unlocked for ->page_mkwrite(). If it lost
> +		 * PG_uptodate meanwhile, retry the fault to read it in again.
> +		 */

The "read it in again" sounds a bit too prescriptive but, apart from that:

Reviewed-by: Pedro Falcato <pfalcato@suse.de>

Thanks!

-- 
Pedro

Re: [PATCH v2 1/2] mm/memory: recheck PG_uptodate after ->page_mkwrite()

[Kiryl Shutsemau]

On Mon, Nov 09, 2026 at 02:11:23PM +0000, Pedro Falcato wrote:
> > diff --git a/mm/memory.c b/mm/memory.c
> > index fe72e66c9c2e..cc1d0b7d15c7 100644
> > --- a/mm/memory.c
> > +++ b/mm/memory.c
> > @@ -5613,6 +5613,18 @@ static vm_fault_t do_anonymous_page(struct vm_fault *vmf)
> >  	return VM_FAULT_OOM;
> >  }
> >  
> > +/*
> > + * A page cache folio that is not uptodate holds data that was never read in
> > + * or cleared, and mapping it leaks stale memory to userspace.
> > + */
> > +static bool file_folio_not_uptodate(struct folio *folio)
> > +{
> > +	if (likely(folio_test_uptodate(folio)))
> > +		return false;
> > +	/* Driver pages from ->fault() never get PG_uptodate */
> > +	return !folio_test_anon(folio) && folio->mapping;
> 
> FYI you can wrap this line in an unlikely() and it has the same effect as
> wrapping every call:
> 	return unlikely(!folio_test_anon(folio) && folio->mapping);

I prefer it at the call site. That is where the reader needs to see
that the retry is the rare path.

> > +}
> > +
> >  /*
> >   * Either the VMA lock or the mmap_lock must have been held on entry
> >   * (see FAULT_FLAG_VMA_LOCK) and may have been released depending on
> > @@ -6163,6 +6175,16 @@ static vm_fault_t do_shared_fault(struct vm_fault *vmf)
> >  			folio_put(folio);
> >  			return tmp;
> >  		}
> > +
> > +		/*
> > +		 * The folio was unlocked for ->page_mkwrite(). If it lost
> > +		 * PG_uptodate meanwhile, retry the fault to read it in again.
> > +		 */
> 
> The "read it in again" sounds a bit too prescriptive but, apart from that:

Okay, will drop it if respin needed.

David, could you adjust it on apply otherwise?

> Reviewed-by: Pedro Falcato <pfalcato@suse.de>

Thanks!

-- 
  Kiryl Shutsemau / Kirill A. Shutemov

Re: [PATCH v2 1/2] mm/memory: recheck PG_uptodate after ->page_mkwrite()

[Lorenzo Stoakes (ARM)]

On Mon, Nov 09, 2026 at 12:04:01PM +0000, Kiryl Shutsemau (Meta) wrote:
> do_shared_fault() unlocks the folio around ->page_mkwrite(). Writeback
> can lock it in that window, fail, and clear PG_uptodate with the folio
> left in the page cache. ecryptfs_writepages() does this when encryption
> fails. filemap_page_mkwrite() relocks the folio and checks only
> ->mapping, so the fault maps a folio that is not uptodate:
>
>     CPU0                            CPU1
>     do_shared_fault()
>       folio_unlock()
>                                     ecryptfs_writepages()
>                                       folio_clear_uptodate()
>       filemap_page_mkwrite()
>         folio_lock()
>       finish_fault()
>
> The next read(2) or fault on the folio reads it in again from the file,
> over anything stored through the mapping in the meantime. Those stores
> are lost.
>
> Recheck PG_uptodate once ->page_mkwrite() returns with the folio locked,
> and retry the fault if it is clear, the same way filemap_fault() retries
> on a cached folio that lost PG_uptodate. Skip folios outside the page
> cache: driver pages from ->fault() never get PG_uptodate and would
> retry forever.
>
> The check stays out of do_page_mkwrite(): wp_page_shared() calls it on
> a folio that is still mapped, and a retry there would fault on the same
> PTE forever.
>
> Reported-by: Sashiko <sashiko-bot@kernel.org>
> Closes: https://lore.kernel.org/sashiko-outbox-202966@kernel.org/

No Fixes? Sorry I may have missed context from v1 here :)

> Signed-off-by: Kiryl Shutsemau (Meta) <kas@kernel.org>
> ---
>  mm/memory.c | 22 ++++++++++++++++++++++
>  1 file changed, 22 insertions(+)
>
> diff --git a/mm/memory.c b/mm/memory.c
> index fe72e66c9c2e..cc1d0b7d15c7 100644
> --- a/mm/memory.c
> +++ b/mm/memory.c
> @@ -5613,6 +5613,18 @@ static vm_fault_t do_anonymous_page(struct vm_fault *vmf)
>  	return VM_FAULT_OOM;
>  }
>
> +/*
> + * A page cache folio that is not uptodate holds data that was never read in
> + * or cleared, and mapping it leaks stale memory to userspace.
> + */
> +static bool file_folio_not_uptodate(struct folio *folio)
> +{
> +	if (likely(folio_test_uptodate(folio)))
> +		return false;
> +	/* Driver pages from ->fault() never get PG_uptodate */

I guess you mean drivers that return non-page cache folios from ->fault()?
Because the driver could return page cache folios there (e.g. shmem), in
which case folio->mapping would be set (set on being added to the page
cache).

But also things are complicated by DAX (of course) which _does_ set
folio->mapping but installs the folio itself so won't reach the test
anyway.

So I think 'rmappable' is the better thing to say given DAX and, given this
is fiddly, maybe worth explicitly pointing out that ->fault might return
non-rmappable folios, e.g.:

	/*
	 * Drivers which specify ->fault() might return non-rmappable folios
	 * (in which case no PG_uptodate).
	 */

> +	return !folio_test_anon(folio) && folio->mapping;

Technically folio_test_anon() can never return true based on where this is
called from right? Could this just be a:

	VM_WARN_ON_ONCE(folio_test_anon(folio));
	/* ... comment */
	return folio->mapping;

Instead?

(Note that __folio_add_file_rmap() asserts on folio_test_anon(folio)).

> +}
> +
>  /*
>   * Either the VMA lock or the mmap_lock must have been held on entry
>   * (see FAULT_FLAG_VMA_LOCK) and may have been released depending on
> @@ -6163,6 +6175,16 @@ static vm_fault_t do_shared_fault(struct vm_fault *vmf)
>  			folio_put(folio);
>  			return tmp;
>  		}
> +
> +		/*
> +		 * The folio was unlocked for ->page_mkwrite(). If it lost
> +		 * PG_uptodate meanwhile, retry the fault to read it in again.
> +		 */
> +		if (unlikely(file_folio_not_uptodate(folio))) {

As Pedro said, I think this unlikely() is probably unnecessary given the true
return case is implicitly unlikely.

> +			folio_unlock(folio);
> +			folio_put(folio);
> +			return VM_FAULT_NOPAGE;
> +		}
>  	}
>
>  	ret |= finish_fault(vmf);
> --
> 2.54.0
>

--
Cheers, Lorenzo

Re: [PATCH v2 1/2] mm/memory: recheck PG_uptodate after ->page_mkwrite()

[Kiryl Shutsemau]

On Mon, Nov 09, 2026 at 04:11:40PM +0000, Lorenzo Stoakes (ARM) wrote:
> On Mon, Nov 09, 2026 at 12:04:01PM +0000, Kiryl Shutsemau (Meta) wrote:
> > do_shared_fault() unlocks the folio around ->page_mkwrite(). Writeback
> > can lock it in that window, fail, and clear PG_uptodate with the folio
> > left in the page cache. ecryptfs_writepages() does this when encryption
> > fails. filemap_page_mkwrite() relocks the folio and checks only
> > ->mapping, so the fault maps a folio that is not uptodate:
> >
> >     CPU0                            CPU1
> >     do_shared_fault()
> >       folio_unlock()
> >                                     ecryptfs_writepages()
> >                                       folio_clear_uptodate()
> >       filemap_page_mkwrite()
> >         folio_lock()
> >       finish_fault()
> >
> > The next read(2) or fault on the folio reads it in again from the file,
> > over anything stored through the mapping in the meantime. Those stores
> > are lost.
> >
> > Recheck PG_uptodate once ->page_mkwrite() returns with the folio locked,
> > and retry the fault if it is clear, the same way filemap_fault() retries
> > on a cached folio that lost PG_uptodate. Skip folios outside the page
> > cache: driver pages from ->fault() never get PG_uptodate and would
> > retry forever.
> >
> > The check stays out of do_page_mkwrite(): wp_page_shared() calls it on
> > a folio that is still mapped, and a retry there would fault on the same
> > PTE forever.
> >
> > Reported-by: Sashiko <sashiko-bot@kernel.org>
> > Closes: https://lore.kernel.org/sashiko-outbox-202966@kernel.org/
> 
> No Fixes? Sorry I may have missed context from v1 here :)

The window is from 2007, when __do_fault() started to drop the folio
lock around ->page_mkwrite():

Fixes: 6967614761fd ("ocfs2: release page lock before calling ->page_mkwrite")

Not sure how useful such Fixes is. It predates any possible stable tree.

> > ---
> >  mm/memory.c | 22 ++++++++++++++++++++++
> >  1 file changed, 22 insertions(+)
> >
> > diff --git a/mm/memory.c b/mm/memory.c
> > index fe72e66c9c2e..cc1d0b7d15c7 100644
> > --- a/mm/memory.c
> > +++ b/mm/memory.c
> > @@ -5613,6 +5613,18 @@ static vm_fault_t do_anonymous_page(struct vm_fault *vmf)
> >  	return VM_FAULT_OOM;
> >  }
> >
> > +/*
> > + * A page cache folio that is not uptodate holds data that was never read in
> > + * or cleared, and mapping it leaks stale memory to userspace.
> > + */
> > +static bool file_folio_not_uptodate(struct folio *folio)
> > +{
> > +	if (likely(folio_test_uptodate(folio)))
> > +		return false;
> > +	/* Driver pages from ->fault() never get PG_uptodate */
> 
> I guess you mean drivers that return non-page cache folios from ->fault()?

Yes. How about:

        /*
         * ->fault() can also return driver memory that is not in the page
         * cache. It never gets PG_uptodate.
         */

I would rather avoid "rmappable". These folios do get
folio_add_file_rmap_ptes() in set_pte_range(), and the word reads like
PG_large_rmappable. As you said, DAX never reaches the check, so "page
cache" is accurate.

> > +	return !folio_test_anon(folio) && folio->mapping;
> 
> Technically folio_test_anon() can never return true based on where this is
> called from right? Could this just be a:
> 
> 	VM_WARN_ON_ONCE(folio_test_anon(folio));
> 	/* ... comment */
> 	return folio->mapping;
> 
> Instead?

Right, all callers pass a file folio.

I will make it an assert, but at the top of the helper, so it checks
every call and not only the rare !uptodate one.

> > @@ -6163,6 +6175,16 @@ static vm_fault_t do_shared_fault(struct vm_fault *vmf)
> >  			folio_put(folio);
> >  			return tmp;
> >  		}
> > +
> > +		/*
> > +		 * The folio was unlocked for ->page_mkwrite(). If it lost
> > +		 * PG_uptodate meanwhile, retry the fault to read it in again.
> > +		 */
> > +		if (unlikely(file_folio_not_uptodate(folio))) {
> 
> As Pedro said, I think this unlikely() is probably unnecessary given the true
> return case is implicitly unlikely.

As I told Pedro, I prefer it at the call site. That is where the reader
needs to see that the retry is the rare path. But if you insist...

-- 
  Kiryl Shutsemau / Kirill A. Shutemov

Re: [PATCH v2 1/2] mm/memory: recheck PG_uptodate after ->page_mkwrite()

[Lorenzo Stoakes (ARM)]

On Mon, Nov 09, 2026 at 05:17:03PM +0000, Kiryl Shutsemau wrote:
> On Mon, Nov 09, 2026 at 04:11:40PM +0000, Lorenzo Stoakes (ARM) wrote:
> > On Mon, Nov 09, 2026 at 12:04:01PM +0000, Kiryl Shutsemau (Meta) wrote:
> > > do_shared_fault() unlocks the folio around ->page_mkwrite(). Writeback
> > > can lock it in that window, fail, and clear PG_uptodate with the folio
> > > left in the page cache. ecryptfs_writepages() does this when encryption
> > > fails. filemap_page_mkwrite() relocks the folio and checks only
> > > ->mapping, so the fault maps a folio that is not uptodate:
> > >
> > >     CPU0                            CPU1
> > >     do_shared_fault()
> > >       folio_unlock()
> > >                                     ecryptfs_writepages()
> > >                                       folio_clear_uptodate()
> > >       filemap_page_mkwrite()
> > >         folio_lock()
> > >       finish_fault()
> > >
> > > The next read(2) or fault on the folio reads it in again from the file,
> > > over anything stored through the mapping in the meantime. Those stores
> > > are lost.
> > >
> > > Recheck PG_uptodate once ->page_mkwrite() returns with the folio locked,
> > > and retry the fault if it is clear, the same way filemap_fault() retries
> > > on a cached folio that lost PG_uptodate. Skip folios outside the page
> > > cache: driver pages from ->fault() never get PG_uptodate and would
> > > retry forever.
> > >
> > > The check stays out of do_page_mkwrite(): wp_page_shared() calls it on
> > > a folio that is still mapped, and a retry there would fault on the same
> > > PTE forever.
> > >
> > > Reported-by: Sashiko <sashiko-bot@kernel.org>
> > > Closes: https://lore.kernel.org/sashiko-outbox-202966@kernel.org/
> >
> > No Fixes? Sorry I may have missed context from v1 here :)
>
> The window is from 2007, when __do_fault() started to drop the folio
> lock around ->page_mkwrite():
>
> Fixes: 6967614761fd ("ocfs2: release page lock before calling ->page_mkwrite")

Yeah not insisting!

>
> Not sure how useful such Fixes is. It predates any possible stable tree.
>
> > > ---
> > >  mm/memory.c | 22 ++++++++++++++++++++++
> > >  1 file changed, 22 insertions(+)
> > >
> > > diff --git a/mm/memory.c b/mm/memory.c
> > > index fe72e66c9c2e..cc1d0b7d15c7 100644
> > > --- a/mm/memory.c
> > > +++ b/mm/memory.c
> > > @@ -5613,6 +5613,18 @@ static vm_fault_t do_anonymous_page(struct vm_fault *vmf)
> > >  	return VM_FAULT_OOM;
> > >  }
> > >
> > > +/*
> > > + * A page cache folio that is not uptodate holds data that was never read in
> > > + * or cleared, and mapping it leaks stale memory to userspace.
> > > + */
> > > +static bool file_folio_not_uptodate(struct folio *folio)
> > > +{
> > > +	if (likely(folio_test_uptodate(folio)))
> > > +		return false;
> > > +	/* Driver pages from ->fault() never get PG_uptodate */
> >
> > I guess you mean drivers that return non-page cache folios from ->fault()?
>
> Yes. How about:
>
>         /*
>          * ->fault() can also return driver memory that is not in the page
>          * cache. It never gets PG_uptodate.
>          */
>
> I would rather avoid "rmappable". These folios do get
> folio_add_file_rmap_ptes() in set_pte_range(), and the word reads like
> PG_large_rmappable. As you said, DAX never reaches the check, so "page
> cache" is accurate.

OK, though my concern is you're making an assumption based on where this is
called from, somebody else could use the function somewhere else and... :)

But this is nitpicking really, that's fine!

>
> > > +	return !folio_test_anon(folio) && folio->mapping;
> >
> > Technically folio_test_anon() can never return true based on where this is
> > called from right? Could this just be a:
> >
> > 	VM_WARN_ON_ONCE(folio_test_anon(folio));
> > 	/* ... comment */
> > 	return folio->mapping;
> >
> > Instead?
>
> Right, all callers pass a file folio.
>
> I will make it an assert, but at the top of the helper, so it checks
> every call and not only the rare !uptodate one.

Thanks!

>
> > > @@ -6163,6 +6175,16 @@ static vm_fault_t do_shared_fault(struct vm_fault *vmf)
> > >  			folio_put(folio);
> > >  			return tmp;
> > >  		}
> > > +
> > > +		/*
> > > +		 * The folio was unlocked for ->page_mkwrite(). If it lost
> > > +		 * PG_uptodate meanwhile, retry the fault to read it in again.
> > > +		 */
> > > +		if (unlikely(file_folio_not_uptodate(folio))) {
> >
> > As Pedro said, I think this unlikely() is probably unnecessary given the true
> > return case is implicitly unlikely.
>
> As I told Pedro, I prefer it at the call site. That is where the reader
> needs to see that the retry is the rare path. But if you insist...

You could also remove the likely() in the function itself ;)

>
> --
>   Kiryl Shutsemau / Kirill A. Shutemov

--
Cheers, Lorenzo

[PATCH v2 2/2] mm/memory: refuse to map page cache folios that are not uptodate

[Kiryl Shutsemau (Meta)]

A page cache folio that is not uptodate holds data that was never read
in or cleared. shmem fallocate(2), for one, adds such folios and clears
them only on first use. Mapping one hands userspace whatever the memory
held before.

Each path that maps page cache checks for this itself: fault-around in
next_uptodate_folio(), a fault in filemap_fault() or shmem_get_folio().
Nothing in the mapping code does, so a caller that misses the check
leaks stale memory silently. MADV_COLLAPSE did exactly that with
fallocated shmem THPs until recently.

Warn and refuse the mapping in __do_fault() and do_set_pmd().

__do_fault() checks the folio ->fault() returned while it is still
locked. do_shared_fault() unlocks it for ->page_mkwrite(), and a
writeback error can clear PG_uptodate after that; that case retries the
fault instead. For CoW, __do_fault() sees the page cache folio that
do_cow_fault() copies from. do_set_pmd() covers MADV_COLLAPSE, which
maps without a fault.

The fault gets SIGBUS, as for a folio that fails to read in.
MADV_COLLAPSE fails and leaves the range to be faulted in.

Suggested-by: David Hildenbrand (Arm) <david@kernel.org>
Signed-off-by: Kiryl Shutsemau (Meta) <kas@kernel.org>
---
 mm/memory.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/mm/memory.c b/mm/memory.c
index cc1d0b7d15c7..2324c04d632f 100644
--- a/mm/memory.c
+++ b/mm/memory.c
@@ -5684,6 +5684,13 @@ static vm_fault_t __do_fault(struct vm_fault *vmf)
 	else
 		VM_BUG_ON_PAGE(!folio_test_locked(folio), vmf->page);
 
+	if (WARN_ON_ONCE(file_folio_not_uptodate(folio))) {
+		folio_unlock(folio);
+		folio_put(folio);
+		vmf->page = NULL;
+		return VM_FAULT_SIGBUS;
+	}
+
 	return ret;
 }
 
@@ -5727,6 +5734,9 @@ vm_fault_t do_set_pmd(struct vm_fault *vmf, struct folio *folio, struct page *pa
 		return ret;
 	page = &folio->page;
 
+	if (WARN_ON_ONCE(file_folio_not_uptodate(folio)))
+		return VM_FAULT_SIGBUS;
+
 	/*
 	 * Just backoff if any subpage of a THP is corrupted otherwise
 	 * the corrupted page may mapped by PMD silently to escape the
-- 
2.54.0

Re: [PATCH v2 2/2] mm/memory: refuse to map page cache folios that are not uptodate

[Pedro Falcato]

On Mon, Nov 09, 2026 at 12:04:02PM +0000, Kiryl Shutsemau (Meta) wrote:
> A page cache folio that is not uptodate holds data that was never read
> in or cleared. shmem fallocate(2), for one, adds such folios and clears
> them only on first use. Mapping one hands userspace whatever the memory
> held before.
> 
> Each path that maps page cache checks for this itself: fault-around in
> next_uptodate_folio(), a fault in filemap_fault() or shmem_get_folio().
> Nothing in the mapping code does, so a caller that misses the check
> leaks stale memory silently. MADV_COLLAPSE did exactly that with
> fallocated shmem THPs until recently.
> 
> Warn and refuse the mapping in __do_fault() and do_set_pmd().
> 
> __do_fault() checks the folio ->fault() returned while it is still
> locked. do_shared_fault() unlocks it for ->page_mkwrite(), and a
> writeback error can clear PG_uptodate after that; that case retries the
> fault instead. For CoW, __do_fault() sees the page cache folio that
> do_cow_fault() copies from. do_set_pmd() covers MADV_COLLAPSE, which
> maps without a fault.
> 
> The fault gets SIGBUS, as for a folio that fails to read in.
> MADV_COLLAPSE fails and leaves the range to be faulted in.
> 
> Suggested-by: David Hildenbrand (Arm) <david@kernel.org>
> Signed-off-by: Kiryl Shutsemau (Meta) <kas@kernel.org>

Awesome!

Reviewed-by: Pedro Falcato <pfalcato@suse.de>

-- 
Pedro