On Mon, Oct 19, 2026 at 1:53 PM Matthew Wilcox <[email protected]> wrote:
>
> On Fri, Oct 02, 2026 at 02:12:35PM +0100, Lorenzo Stoakes (ARM) wrote:
> > On Thu, Oct 01, 2026 at 10:12:16PM +0100, Matthew Wilcox wrote:
> > > So what I didn't realise is that fork() waits for page faults to finish.
> > > I don't think that's necessary, so we can just stop doing that (whitespace
> > > damaged):
> > > I think this is safe. I've booted a kernel with this change, and
> >
> > I'm afraid it's not safe :)
> >
> > We already actually did this (by mistake I think?) before. See commit
> > fb49c455323f ("fork: lock VMAs of the parent process when forking") - we
> > shipped
> > that in 6.4.0 -> 6.4.2 and it corrupted memory all over the place.
> >
> > Reports + a consistent reproducer:
> >
> > https://bugzilla.kernel.org/show_bug.cgi?id=217624
> > https://lore.kernel.org/all/[email protected]/
> > https://lore.kernel.org/all/[email protected]/
>
> I wonder if we still need this. The commit message says:
>
> A concurrent page fault on a page newly marked read-only by the page
> copy might trigger wp_page_copy() and a anon_vma_prepare(vma) on the
> source vma, defeating the anon_vma_clone() that wasn't done because the
> parent vma originally didn't have an anon_vma, but we now might end up
> copying a pte entry for a page that has one.
>
> But we're holding the mmap_lock for write here, and we need to hold the
> mmap_lock for read in order to change a vma->anon_vma (asserted in
> __anon_vma_prepare() with mmap_assert_locked()).
>
> At the time of this commit, lock_vma_under_rcu() contained:
>
> /* find_mergeable_anon_vma uses adjacent vmas which are not locked */
> if (!vma->anon_vma && !vma_is_tcp(vma))
> goto inval;
>
> so we really did have to wait for read holders to finish the fault.
> But commit a373baed5a9d deferred the check for a NULL anon VMA until we
> actually needed it, so I think commit fb49c455323f's logic was invalidated
> by a373baed5a9d.
>
> Maybe I'm wrong. This is all a bit out of my normal area.
>
> > Also we have asserts for this lock in copy_page_range() and
> > copy_hugetlb_page_range() which will now fail (and possibly more further
> > down
> > the stack?)
>
> Possibly we have to adjust some assertions to check that either we have
> the VMA locked or we have the MM write locked. That doesn't seem
> unreasonable.
>
> > > My argument for why it's safe is that a thread which takes a page
> > > fault during fork() might have taken the page fault either before or
> > > after fork(). The faults will definitely happen in the parent process.
> > > They may or may not have happened in the child process, which can't
> > > possibly care whether or not they've happened.
> >
> > The problem isn't the child it's the parent - copy_page_range() _modifies_
> > the
> > _parent_'s page tables in-place, on assumption that nothing else can access
> > them (for CoW).
>
> I was perhaps unclear. What I mean is that the child may or may not
> observe them as having happened.
>
> > And actually nowadays things are _even_ worse, you could also have
> > concurrent
> > page faults, MADV_DONTNEED, MADV_FREE, MADV_GUARD_INSTALL/REMOVE,
> > UFFDIO_COPY +
> > UFFDIO_MOVE (yikes) all of which might introduce new issues + would have to
> > be
> > audited too.
>
> Yup.
>
> > So - it turns out a lot of stuff implicitly assumes VMA write lock now:
> >
> > 1. CoW - deferred TLB flush race
> >
> > (The issue above)
> >
> > https://lore.kernel.org/all/[email protected]/
> >
> > This happens because __copy_present_ptes() does wrprotect_ptes() on the
> > parent
> > without a TLB flush (that's deferred until the end of the fork).
> >
> > But unfortunately without a VMA write lock something can then replace the
> > page
> > table entry in the meantime and you can end up with data loss as a result.
>
> Is it that fb49c455323f commit message is insufficient and we need to
> prevent simultaneous fork/page fault for other reasons, then?
Yes, I think so. It refers to one of the reasons VMA lock is needed
during fork, but as Lorenzo points out, there are a number of other
reasons that require fork to protect VMAs. Note that some of those
reasons, like MADV_GUARD_INSTALL and UFFDIO_MOVE did not exist at the
time fb49c455323f was written.
Below is the list that AI pointed out, and I think Lorenzo covered
most if not all of them:
Without vma_start_write_killable(mpnt):
1. In-flight VMA readers that locked mpnt before
mmap_write_lock_killable(oldmm) continue running concurrently with
dup_mmap().
2. New VMA readers (page faults via lock_vma_under_rcu(),
madvise under MADVISE_VMA_READ_LOCK, userfaultfd via uffd_lock_vma() /
uffd_move_lock(), vma_start_read_unlocked(), etc.) continue to succeed
on mpnt while dup_mmap() duplicates its VMA metadata and page tables.
(Note: When this lock was originally added in fb49c455323f, one
prominent trigger was a concurrent fault calling anon_vma_prepare() on
a VMA with mpnt->anon_vma == NULL while anon_vma_fork() /
vma_needs_copy() ran. Although __vmf_anon_prepare() now requires
mmap_read_trylock()
when !vma->anon_vma, removing vma_start_write_killable(mpnt) today
still breaks several critical invariants.)
──────
### What would happen if we removed it
1. **Destruction of live 2MB THP mappings ("bad pmd" -> pmd_clear())
in copy_pmd_range()**
As noted in the comment in copy_pte_range(), copy_pmd_range()
assumes no concurrent THP transitions on src_pmd:
if (pmd_is_huge(*src_pmd)) { ... }
if (pmd_none_or_clear_bad(src_pmd))
continue;
On an anonymous VMA that already has mpnt->anon_vma != NULL, a
concurrent per-VMA-locked THP fault (do_huge_pmd_anonymous_page()) or
UFFDIO_MOVE can install a transparent huge PMD into *src_pmd right
after pmd_is_huge(*src_pmd) observed pmd_none.
pmd_none_or_clear_bad(src_pmd)
then sees pmd_bad(*src_pmd) == true (a THP leaf PMD satisfies
pmd_bad()), prints "bad pmd ...", and calls pmd_clear_bad(src_pmd) —
wiping out the parent's live 2MB PMD without PTL, TLB flush, or
rmap/refcount/deposited-pgtable/RSS cleanup (corrupting parent memory
and
triggering Bad rss-counter / nr_ptes bugs on exit).
• The same race exists for huge PUDs in copy_pud_range()
(pud_none_or_clear_bad(src_pud)).
• Similarly, copy_huge_pmd() reads special huge PMDs
(VM_PFNMAP/VM_MIXEDMAP) locklessly and skips rechecking *src_pmd under
src_ptl assuming it cannot change, so a concurrent MADV_DONTNEED under
MADVISE_VMA_READ_LOCK could leave the child mapping a stale/zapped PFN
PMD.
2. **Silent CoW data corruption / lost parent writes due to deferred
flush_tlb_mm(oldmm)**
__copy_present_ptes(), copy_huge_pmd(), and
copy_hugetlb_page_range() write-protect parent PTEs/PMDs without
flushing TLBs; dup_mmap() defers the flush to flush_tlb_mm(oldmm)
after all VMAs are copied. Normally, because each copied VMA remains
VMA-write-locked until
mmap_write_unlock(oldmm), no parent thread can handle a CoW write
fault before flush_tlb_mm(oldmm) finishes.
Without the VMA write lock, while CPU 0 is still in dup_mmap()
copying later VMAs:
• CPU 1 (parent thread A) still holds a stale writable TLB entry
to old_folio (now CoW-shared with the child).
• CPU 2 (parent thread B, no cached TLB entry) writes to the
same page, faults on the R/O PTE under per-VMA read lock, enters
wp_page_copy(), and copies old_folio into new_folio
(__wp_page_copy_user()) before taking PTL and calling
ptep_clear_flush().
• Any store by CPU 1 through its stale writable TLB entry after
__wp_page_copy_user() and before ptep_clear_flush() lands directly in
old_folio while pte_same() still succeeds — losing thread A's write in
the parent (new_folio) and leaking it into the child (old_folio).
3. Broken guard-page (MADV_GUARD_INSTALL) state and KCSAN race on vm_flags
madvise(MADV_GUARD_INSTALL) runs under MADVISE_VMA_READ_LOCK and
atomically sets VMA_MAYBE_GUARD_BIT on mpnt via vma_set_atomic_flag()
before installing PTE_MARKER_GUARD entries:
• vm_area_dup(mpnt) asserts
ASSERT_EXCLUSIVE_WRITER(orig->vm_flags), which would trip under KCSAN.
• If vm_area_dup(mpnt) copies mpnt->vm_flags just before
MADV_GUARD_INSTALL sets VMA_MAYBE_GUARD_BIT and installs
PTE_MARKER_GUARD PTEs:
• On a file-backed VMA without anon_vma, tmp misses
VM_MAYBE_GUARD (part of VM_COPY_ON_FORK), so vma_needs_copy() returns
false and drops the guard regions in the child.
• On a VMA with anon_vma != NULL, copy_page_range() copies
the PTE_MARKER_GUARD entries into tmp, leaving the child VMA with
guard PTE markers without VM_MAYBE_GUARD set (breaking collapse checks
such as file_backed_vma_is_retractable() and future merges/forks).
4. Inconsistent address-space snapshots against UFFDIO_MOVE and
tcp_zerocopy_receive()
• When UFFD_FEATURE_EVENT_FORK is not enabled, dup_userfaultfd()
never increments octx->mmap_changing. Because move_pages()
(UFFDIO_MOVE) only takes per-VMA read locks (uffd_move_lock()), it
could move an exclusive folio from a not-yet-copied src address/VMA
into an
already-copied dst address/VMA (losing the page at both
addresses in the child), or move a zero page from an already-copied
src to a not-yet-copied dst (duplicating it at both). Even with
UFFD_FEATURE_EVENT_FORK, octx->mmap_changing is only incremented once
dup_mmap()
reaches the first VMA using octx, so earlier non-uffd VMAs rely
on VMA write-locking to prevent cross-VMA moves.
• tcp_mmap() sets VM_MIXEDMAP (in VM_COPY_ON_FORK, without
VM_DONTCOPY), so copy_page_range() would race with
tcp_zerocopy_receive() zapping and inserting pages under
vma_start_read_unlocked().
5. Immediate debug assertions (CONFIG_DEBUG_VM)
Both copy_page_range() and copy_hugetlb_page_range() call
vma_assert_write_locked(src_vma) on every CoW VMA, which immediately
fires VM_WARN_ON_ONCE_VMA(!__is_vma_write_locked(vma), vma) because
oldmm->mm_lock_seq was bumped by mmap_write_lock_killable(oldmm) while
mpnt-
>vm_lock_seq was never updated.
>
> > Same issue exists with hugetlb wp too. And god only knows with uffd wp :)
> >
> > 2. THP - can destroy parent THP mappings
> >
> > copy_pmd_range() does:
> >
> > if (pmd_is_huge(*src_pmd)) { copy_huge_pmd(); ... }
> > if (pmd_none_or_clear_bad(src_pmd))
> > continue;
> >
> > Without the write lock, a concurrent fault can install a huge PMD via
> > do_huge_pmd_anonymous_page() or filemap_map_pmd()/do_set_pmd().
> >
> > The issue is that pmd_bad() fires for leaf PMD (ugh) on at least x86 and
> > arm64,
> > meaning that'll clear the page table entry.
> >
> > I think Hugh left a comment alluding to this before:
> >
> > * copy_pmd_range()'s prior pmd_none_or_clear_bad(src_pmd), and the
> > * error handling here, assume that exclusive mmap_lock on dst and src
> > * protects anon from unexpected THP transitions; ...
> >
> > It's like the mess around pmd_trans_unstable() :(
>
> ... could we fix this by taking the ptlock during the copy operation?
>
> > 3. copy_pte_range() skips the required pmd_same() recheck assuming write
> > lock
> >
> > It uses pte_offset_map_rw_nolock() with a dummy pmdval and:
> >
> > * We already hold the exclusive mmap_lock, the copy_pte_range() and
> > * retract_page_tables() are using vma->anon_vma to be exclusive, so
> > * the PTE page is stable, and there is no need to get pmdval and do
> > * pmd_same() check.
> >
> > (Probably needs updating for VMA write lock...)
> >
> > 4. The asserts
> >
> > Mentioned above!
> >
> > I suspect there's more as well.
> >
> > Some of it could be changed, but it'd all be at the cost of behaving very
> > strangely (no VMA lock but mmap write lock) while manipulating things.
> >
> > And I am not really happy with changing page table entries in the parent VMA
> > (for CoW) without a VMA write lock held.
> >
> > I also wonder whether it would achieve all that much really - the issue is
> > with
> > holding the VMA lock across I/O, anything else that tries to grab a write
> > lock
> > is going to end up being the new thing that's blocked (mprotect, munmap,
> > mremap,
> > mlock etc.)
> >
> > I think doing something like this would need a very significant redesign
> > and I'm
> > not convinced it's one that really achieves what we want given all the other
> > cases that'd get blocked by the VMA lock being held still.
> >
> > --
> > Cheers, Lorenzo
