Re: [PATCH v5 03/18] userfaultfd: gate must_wait writability check on pte_present()

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] userfaultfd_must_wait() and userfaultfd_huge_must_wait() cause a thread to sleep infinitely if it encounters a poison marker.
--

commit bd3d83c0669b044a367277284d21f8c9e82fd131
Author: Kiryl Shutsemau (Meta) <kas@kernel.org>

userfaultfd: gate must_wait writability check on pte_present()

This commit gates the writability checks in userfaultfd_must_wait() and
userfaultfd_huge_must_wait() on pte_present() to avoid incorrectly
evaluating random offset bits from swap or migration entries as present
PTE flags.

Cc: stable@vger.kernel.org

> diff --git a/mm/userfaultfd.c b/mm/userfaultfd.c
> index 35b206cc9aa6f..f6d2a1c67019e 100644
> --- a/mm/userfaultfd.c
> +++ b/mm/userfaultfd.c
> @@ -2535,6 +2535,15 @@ static inline bool userfaultfd_huge_must_wait(struct userfaultfd_ctx *ctx,
>  	/* UFFD PTE markers require userspace to resolve the fault. */
>  	if (pte_is_uffd_marker(pte))
>  		return true;

[Severity: High]
This is a pre-existing issue, but does this logic cause a thread to sleep
infinitely if it encounters a poison marker?

When a missing fault is resolved by userspace using UFFDIO_POISON, a
PTE_MARKER_POISONED swap entry is installed and the waitqueue is woken up.
If the faulting thread is preempted and misses this wakeup (i.e., it is not
yet on the waitqueue), it will later add itself to the waitqueue and call
userfaultfd_huge_must_wait() to check if the fault was resolved.

Because pte_is_uffd_marker() returns true for poison markers, this
returns true and the thread stays asleep indefinitely. Shouldn't this return
false for poison markers so the thread can retry the fault and receive
a VM_FAULT_HWPOISON -> SIGBUS?

> +	/*
> +	 * Concurrent migration may have replaced the present PTE with a
> +	 * non-marker swap entry between fault delivery and this lockless

[ ... ]

> @@ -2621,6 +2630,17 @@ static inline bool userfaultfd_must_wait(struct userfaultfd_ctx *ctx,
>  	/* UFFD PTE markers require userspace to resolve the fault. */
>  	if (pte_is_uffd_marker(ptent))
>  		goto out;

[Severity: High]
This is a pre-existing issue, but similarly here, does returning true via
the goto out for a poison marker cause an indefinite hang if the thread
missed the UFFDIO_POISON wakeup?

> +	/*
> +	 * Concurrent swap-out / migration may have replaced the present PTE
> +	 * with a non-marker swap entry between fault delivery and this
> +	 * lockless re-check. pte_write() on a swap entry decodes random

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260526130509.2748441-1-kirill@shutemov.name?part=3