sashiko.dev --
https://sashiko.dev/#/patchset/[email protected] -- wrote:
> Since ctx->mm can be an external mm_struct, is it possible for the target
> process to have encountered an OOM-reap or a failed dup_mmap() and be
> marked MMF_UNSTABLE?
> If so, should there be a call to check_stable_address_space(mm) after
> acquiring the mmap lock to avoid iterating over a maple tree that might
> contain XA_ZERO_ENTRY markers?
This is the same pattern as userfaultfd_register() and
userfaultfd_unregister(), which acquire mmap_write_lock(mm) after a
successful mmget_not_zero() and walk the VMA tree without
check_stable_address_space().
The OOM reaper takes mmap_read_lock, so it is excluded once we hold the
write lock; failed dup_mmap() unwinds its partial tree before returning.
> The commit message notes that fdinfo reads ctx->features with READ_ONCE to
> avoid seeing a mid-RMW intermediate value. Are there other lockless readers
> of ctx->features that also need this annotation?
[ ... ]
> Could executing UFFDIO_SET_MODE concurrently with these paths cause a data
> race on ctx->features?
Confirmed. userfaultfd_is_initialized() is reached from
userfaultfd_poll(), userfaultfd_read_iter(), and userfaultfd_ioctl()
with no mm lock held, so SET_MODE's mmap_write_lock + vma_start_write()
drain does not exclude them. The INITIALIZED bit is never modified by
SET_MODE so the value is functionally stable, but READ_ONCE pairing is
still the right thing for KCSAN.
Will fold into 12/14 a small helper plus conversions:
static unsigned int userfaultfd_features(struct userfaultfd_ctx *ctx)
{
return READ_ONCE(ctx->features);
}
with userfaultfd_is_initialized(), userfaultfd_wp_async_ctx(),
userfaultfd_rwp_async_ctx(), userfaultfd_wp_unpopulated(), and the
fdinfo printer reading through the helper. Hot-path reads inside
handle_userfault() and friends stay plain -- they run under the
per-VMA lock or mmap_read_lock that SET_MODE drains before the RMW.
--
Kiryl Shutsemau / Kirill A. Shutemov
