Re: [PATCH] mm/shmem: use invalidate_lock to fix hole-punch race

Gregory Price
2026-03-26T18:37:23Z

On Thu, Mar 26, 2026 at 05:07:42PM +0000, Pedro Falcato wrote:
> > Two races allow PTEs to be re-installed for a folio that fallocate
> > is about to remove from page cache:
>
> Hmm, I don't see how your patch fixes anything.
>

after looking at your comments below i realized race 2 actually requires
the fork as well, which means they're both essentially variations of the
same race, so hopefully i can simplify the change log.

> > fallocate fault-around fork
> > -------- ------------ ----
> > set i_private
> > unmap_mapping_range()
> > # zaps PTEs
> > filemap_map_pages()
> > # re-maps folio!
> > dup_mmap()
> > # child VMA
> > # in tree
> > shmem_undo_range()
> > lock folio
> > unmap_mapping_folio()
^^^ i_mmap_lock_read held, iterates VMAs
> spin_lock(ptl);
^^^ child VMA's PTL
> > # child VMA:
> > # no PTE, skip
> spin_unlock(ptl);
^^^ child VMA done, iterator moves on
it will not re-visit the child.

> > copy_page_range()
> spin_lock(dst_ptl);
^ Child PTL
> spin_lock(src_ptl);
^ Parent PTL
> /* does not copy PTE. either
> * we find a zapped PTE, or unmap_mapping_folio()
> * finds two mappings instead of one. */

At this point, unmap_mapping_folio only processed the child VMA
(no PTE, skip). The parent PTE *has not* been zapped.

copy_page_range() acquires src_ptl (parent) and reads a present PTE,
and boom copies it to child.

When it reaches the parent VMA next, it zaps the parent PTE,
but the child PTE (just installed) survives.

> >
> > Fix both races with invalidate_lock.
> >
>
> I don't see what you're seeing? Note that both map_pages and fault()
> take the folio lock (map_pages does a trylock) to exclude against truncate
> as well.
>

The folio lock serializes map_pages/fault against truncate - but the
race isn't between those two. It's between truncate's unmap walk and
fork's copy_page_range - and copy_page_range doesn't take folio lock.

The easiest way to deal with this is to prevent these fork-inserted PTEs
from existing rather than try to make copy_page_range aware of
truncation (it already holds the PTL when it finds the PTE, so you can't
take the folio lock unless you drop/reacquire the PTL).

~Gregory