On Mon, Oct 26, 2026 at 7:39 PM Suren Baghdasaryan <[email protected]> wrote:
>
> 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.
I also have to agree with Lorenzo that allowing fork to run
concurrently with page faults might only shift the mmap_lock
contention from fork to some other operation (mmap/mprotect/etc.), so
even if we fix all these cases we might end up with contention
somewhere else.
>
> 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
