Re: [PATCH] userfaultfd: clear uffd state in child PTEs if uffd is released during fork

[Tal Zussman <tz2294@columbia.edu>]

On Sat, Oct 31, 2026 at 02:35:05PM +0200, Mike Rapoport (Microsoft) wrote:
> With UFFD_FEATURE_EVENT_FORK the userfaultfd context is duplicated for the
> child process, the child VMAs are registered with the new context, and uffd
> protection state from the parent PTEs is copied into the child.
>
> If the parent's userfaultfd is released before the fork event is read,
> userfaultfd_release_new() resets the VMA flags but keeps uffd state in the
> child PTEs.
>
> This leaves the child's VMAs in an inconsistent state: they are not
> registered with userfaultfd but their mappings carry uffd protection state.
>
> If the child registers the range with userfaultfd again, the stale state
> turns into protection the child never asked for.
>
> When a write fault makes such a PTE writable without copying the page, the
> uffd bit remains set and page_table_check_pte_flags() warns about it.
>
> Split resolving of the uffd PTE state out of userfaultfd_clear_vma() and
> call it from userfaultfd_release_new() before resetting the VMA.
>
> Fixes: f369b07c8614 ("mm/uffd: reset write protection when unregister with wp-mode")
> Cc: stable@vger.kernel.org
> Signed-off-by: Mike Rapoport (Microsoft) <rppt@kernel.org>

Hi Mike,

I managed to trigger a warning without this patch too by using an
mprotect(PROT_READ | PROT_WRITE) in the child with CONFIG_PAGE_TABLE_CHECK:

  WARNING: mm/page_table_check.c:202 at __page_table_check_ptes_set+0x181/0x2f0
  Call Trace:
   change_pte_range+0x1ef7/0x29b0
   change_protection+0xd2b/0x1560
   mprotect_fixup+0x4cb/0x1040
   do_mprotect_pkey+0x7c2/0xac0
   __x64_sys_mprotect+0x78/0xc0

No warning with the patch, so:

Reviewed-by: Tal Zussman <tz2294@columbia.edu>
Tested-by: Tal Zussman <tz2294@columbia.edu>

With a couple comments about comments below.

> ---
>  mm/userfaultfd.c | 43 ++++++++++++++++++++++++++-----------------
>  1 file changed, 26 insertions(+), 17 deletions(-)
>
> diff --git a/mm/userfaultfd.c b/mm/userfaultfd.c
> index 3b71712edab4c..2c11ac5dad3d5 100644
> --- a/mm/userfaultfd.c
> +++ b/mm/userfaultfd.c
> @@ -2220,6 +2220,28 @@ static void userfaultfd_reset_ctx(struct vm_area_struct *vma)
>  	userfaultfd_set_ctx(vma, NULL, 0);
>  }
>  

I think uffd is woefully under-commented considering how many edge cases and
races are associated with it... so how about the following instead:

/*
 * Clear uffd state from a WP or RWP range: the uffd bit, WP markers and
 * RWP PAGE_NONE entries. Call this before dropping the registration, as it
 * needs the VMA flags to know what to resolve.
 */

> +/* Clear the uffd bit and/or restore protnone PTEs */
> +static void userfaultfd_clear_ptes(struct vm_area_struct *vma,
> +		unsigned long start, unsigned long end)
> +{
> +	unsigned int mm_cp_flags = 0;
> +	struct mmu_gather tlb;
> +
> +	if (!userfaultfd_protected(vma))
> +		return;
> +
> +	if (userfaultfd_wp(vma))
> +		mm_cp_flags |= MM_CP_UFFD_WP_RESOLVE;
> +	if (userfaultfd_rwp(vma))
> +		mm_cp_flags |= MM_CP_UFFD_RWP_RESOLVE;
> +	if (vma_wants_manual_pte_write_upgrade(vma))
> +		mm_cp_flags |= MM_CP_TRY_CHANGE_WRITABLE;
> +
> +	tlb_gather_mmu(&tlb, vma->vm_mm);
> +	change_protection(&tlb, vma, start, end, mm_cp_flags);
> +	tlb_finish_mmu(&tlb);
> +}

I think it may also be helpful to comment the fork release path that
triggers this bug for clarity, so how about the below on top? Happy to send
it separately.

That ewq->msg.arg.reserved.reserved1 at the end is horrific too... I'll send
a patch to clean that up.

---8<---
From: Tal Zussman <tz2294@columbia.edu>
Subject: [PATCH] mm/userfaultfd: comment the fork event release path

Document what userfaultfd_reset_ctx() expects from its callers, and why
the fork event release path has to unregister the child.

Link: https://lore.kernel.org/lkml/20261031-uffd-prot-bits-fork-event-v1-1-3572921c2111@kernel.org/
Signed-off-by: Tal Zussman <tz2294@columbia.edu>
---
 mm/userfaultfd.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/mm/userfaultfd.c b/mm/userfaultfd.c
index 3b71712edab4..249f3b4444ab 100644
--- a/mm/userfaultfd.c
+++ b/mm/userfaultfd.c
@@ -2215,6 +2215,13 @@ static void userfaultfd_set_ctx(struct vm_area_struct *vma,
 				 (vma->vm_flags & ~__VM_UFFD_FLAGS) | vm_flags);
 }
 
+/*
+ * Drops the registration from the VMA.
+ *
+ * Page tables are left alone. Callers must make sure no uffd state is
+ * left behind in them, either by calling userfaultfd_clear_ptes() first
+ * or because none was ever installed in the range.
+ */
 static void userfaultfd_reset_ctx(struct vm_area_struct *vma)
 {
 	userfaultfd_set_ctx(vma, NULL, 0);
@@ -2997,6 +3004,10 @@ static void userfaultfd_event_wait_completion(struct userfaultfd_ctx *ctx,
 		set_current_state(TASK_KILLABLE);
 		if (ewq->msg.event == 0)
 			break;
+		/*
+		 * The fd is gone or we are being killed, so the event
+		 * will never be read.
+		 */
 		if (READ_ONCE(ctx->released) ||
 		    fatal_signal_pending(current)) {
 			/*
@@ -3009,6 +3020,10 @@ static void userfaultfd_event_wait_completion(struct userfaultfd_ctx *ctx,
 			if (ewq->msg.event == UFFD_EVENT_FORK) {
 				struct userfaultfd_ctx *new;
 
+				/*
+				 * Nobody will get an fd for the new context,
+				 * so unregister the child.
+				 */
 				new = (struct userfaultfd_ctx *)
 					(unsigned long)
 					ewq->msg.arg.reserved.reserved1;
-- 
2.39.5