From: "Lorenzo Stoakes (ARM)" <ljs@kernel.org>
To: Tal Zussman <tz2294@columbia.edu>
Cc: "David Hildenbrand (Arm)" <david@kernel.org>,
Andrew Morton <akpm@linux-foundation.org>,
"Liam R. Howlett" <liam@infradead.org>,
Vlastimil Babka <vbabka@kernel.org>,
Jann Horn <jannh@google.com>, Mike Rapoport <rppt@kernel.org>,
Suren Baghdasaryan <surenb@google.com>,
Michal Hocko <mhocko@suse.com>, Shuah Khan <shuah@kernel.org>,
linux-mm@kvack.org, linux-kernel@vger.kernel.org,
linux-kselftest@vger.kernel.org, stable@vger.kernel.org
Subject: Re: [PATCH] mm/madvise: disallow guard region installation in uffd-armed VMAs
Date: Fri, 13 Nov 2026 21:09:44 +0000 [thread overview]
Message-ID: <avd6XEHndh30hyc-@gremlin> (raw)
In-Reply-To: <f53458fd-c268-4dfe-aa99-b050f63af728@columbia.edu>
On Fri, Nov 13, 2026 at 03:55:41PM -0500, Tal Zussman wrote:
> On 11/13/26 2:22 PM, Lorenzo Stoakes (ARM) wrote:
> > On Fri, Nov 13, 2026 at 08:15:34PM +0100, David Hildenbrand (Arm) wrote:
> >> On 11/13/26 14:59, Lorenzo Stoakes (ARM) wrote:
> >> > On Fri, Nov 13, 2026 at 12:59:00AM +0100, David Hildenbrand (Arm) wrote:
> >> >> On 11/12/26 18:38, Lorenzo Stoakes (ARM) wrote:
> >> >>> In order to establish guard regions, guard markers are installed in the
> >> >>> page tables of a mapped VMA.
> >> >>>
> >> >>> However, problems arise when one tries to combine this with registering a
> >> >>> VMA for userfaultfd, as such operations may themselves need to install page
> >> >>> table markers such as uffd-wp.
> >> >>
> >> >> How does this interact with UFFDIO_POISON markers that can outlive even
> >> >> userfaultfd registration?
> >> >
> >> > The zap clears poison markers, as is standard. It's really horrible this logic.
> >> >
> >> > So the code path for zap used by the guard install (via zap_vma_range()) is:
> >> >
> >> > zap_vma_range()
> >> > -> zap_vma_range_batched() with NULL zap_details
> >> > -> __zap_vma_range()
> >> > -> zap_p4d_range()
> >> > ...
> >> > -> zap_pte_range()
> >> > -> do_zap_pte_range()
> >> > -> zap_nonpresent_ptes()
> >> >
> >> > And zap_details is NULL.
> >> > > In zap_non_present_ptes():
> >> >
> >> > ...
> >> > } else if (softleaf_is_uffd_wp_marker(entry)) {
> >> > if (... !zap_drop_markers(details))
> >> > return 1;
> >> > } else if (softleaf_is_guard_marker(entry)) {
> >> > ...
> >> > if (!zap_drop_markers(details))
> >> > return 1;
> >> > } else if (softleaf_is_hwpoison(entry) ||
> >> > softleaf_is_poison_marker(entry)) {
> >> > if (!should_zap_cows(details))
> >> > return 1;
> >> > } ...
> >> >
> >> > And zap_drop_markers() checks zap_details && zap_details->flags &
> >> > ZAP_FLAG_DROP_MARKER.
> >>
> >> Right, my intuition tells me to just nuke that thing by setting
> >> ZAP_FLAG_DROP_MARKER.
> >>
> >> MADV_GUARD, is just like munmap() another form of "zap it all and protect it".
> >> >>> This is especially problematic because guard region installation causes the
> >> >>> regions to be zapped first, meaning that the removal of userfaultfd-tracked
> >> >>> ranges will not be logged in this case.
> >> >>
> >> >> Again, similar to trying to uffd-wp-protect'ing a hwpoison marker (which will be
> >> >> ignored), and then zapping that.
> >> >>
> >> >> Isn't guard marker installation/removal just another form of zapping?
> >> >
> >> > I mean users being weird could decide to use it for that rather than
> >> > MADV_DONTNEED, but zapping is a side effect really.
> >> >
> >> > Really the idea is that you might have users who, for whatever reason,
> >> > faulted in some range, and now want to make that a guard range, so for
> >> > convenience we zap it for them :)
> >>
> >> I think not just convenience. There was this problem with faultaround / THP
> >> placement IIRC ...
> >>
> >> I'd just zap everything that's there. That's literally what the user asked for.
> >>
> >> >
> >> >>
> >> >> So to me this all sounds like "if user space does something stupid, it's user
> >> >> space's fault".
> >> >
> >> > Unfortunately, as per Tal's series, there's more to it than that (linked in
> >> > Closes: tags).
> >> >
> >> > You can get an infinite syscall restart (ugh) if you do this on a shmem
> >> > range (Tal's 9/12) so it's a kernel bug that _has_ to be fixed.
> >> >
> >> > The other bits are just about a user losing track of uffd stuff if they
> >> > guard/unguard, as you say.
> >> >
> >> > But every other zap code path does the uffd remove event stuff, so this is
> >> > still an exception.
> >> >
> >> > If you support uffd, you have to add a whole bunch of horrifying, awful,
> >> > terrible open-coded uffd crap, e.g. the userfaultfd_remove() etc. And you
> >> > have to drop the bloody lock and reacquire it too and make sure all that
> >> > works correctly (...!)
> >> >
> >> > Anyway there's basically 3 options here:
> >> >
> >> > 1. do something like what Tal's series does - add a TON of complexity for a
> >> > use case that nobody uses to get behaviour on something nobody sane
> >> > should care about (i.e. uffd/guard/unguard/expect uffd correctness).
> >> >
> >> > 2. Zap with ZAP_FLAG_DROP_MARKER, and silently lose uffd-wp state but
> >> > 'support' uffd. It does fix the 9/12 bug but then subtly breaks uffd.
> >>
> >> Right, that's what I would do.
> >>
> >> MADV_GUARD_INSTALL zaps everything.
> >>
> >> It already zaps swap entries, migration entries, hwpoison entries ... so just
> >> zap everything.
> >>
> >> [...]
> >>
> >> >
> >> > If you're really concerned though, we could go with option 2, which will
> >> > just make UFFD behave differently for guard regions.
> >> >
> >>
> >> Not really concerned, it's just the most consistent one for me. Zap everything.
> >> The user asks for it, like he would ask with munmap().
> >>
> >> > (I regret, obviously, ever allowing this for uffd ranges. And we _all_
> >> > regret that uffd was _ever_ allowed to be merged in this state...)
> >> Yeah ...
> >
> > Ack will send a v2 doing that then :)
> >
> This sounds better to me. My concern in [1] with completely disabling this
> was that it could break a uffd manager that doesn't control the task it
> manages (e.g. CRIU lazy restore registering the memory of a task whose libc
> uses guard regions), but this should avoid that.
But they were completely broken already? So we're 'breaking' an already broken
user? :)
It's moot, but still!
>
> This still leaves guard installation as the one zap path that doesn't send
> a UFFD_EVENT_REMOVE notification, but for CRIU I think that's fine, since
> guards would mostly go on memory that didn't have contents beforehand, so
> there's no stale contents to restore.
For one, CRIU is userland software that relies on kernel implementation details,
so we are not obliged to keep _kernel_ implementation details stable for them :)
For another, relying on uffd state remaining consistent across the
install/remove of the literal 'put a hole in my address space' feature is deeply
dubious.
>
> I could imagine this mattering for something like a userspace far-memory
> library [2] that registers a heap with uffd MISSING mode. If the allocator
> guards a page that the library has paged out, the library is never told
> to drop its copy, and fills the page with the old contents once the guard
> is removed.
Previously: broken
Now: Not permitted.
I think that's fine ;)
>
> I don't think anything does this today, so it's not worth adding the event,
> but I think this limitation should definitely be documented (i.e., that
> MADV_GUARD_INSTALL discards the range and uffd state without notifying uffd)
> for the sake of the poor souls that work with uffd and to prevent any
> spurious LLM reports in the future...
Documenting things doesn't prevent spurious LLM reports,
unfortunately. Nothing stops the schloppers... ignoring is the only answer,
long-term anyway.
But sure, I'll try to update the manpage for the few humans still reading
these things...
>
> [1] https://lore.kernel.org/linux-mm/f76cb2b0-0090-4554-9169-5cfff6a6ca53@columbia.edu/
> [2] https://www.usenix.org/system/files/nsdi25-yelam.pdf
--
Cheers, Lorenzo
next prev parent reply other threads:[~2026-11-13 21:09 UTC|newest]
Thread overview: 10+ messages / expand[flat|nested] mbox.gz Atom feed top
2026-11-12 17:38 [PATCH] mm/madvise: disallow guard region installation in uffd-armed VMAs Lorenzo Stoakes (ARM)
2026-11-12 20:59 ` Liam R. Howlett
2026-11-12 23:59 ` David Hildenbrand (Arm)
2026-11-13 13:59 ` Lorenzo Stoakes (ARM)
2026-11-13 19:15 ` David Hildenbrand (Arm)
2026-11-13 19:22 ` Lorenzo Stoakes (ARM)
2026-11-13 20:55 ` Tal Zussman
2026-11-13 21:09 ` Lorenzo Stoakes (ARM) [this message]
2026-11-13 21:21 ` Tal Zussman
2026-11-13 8:58 ` Vlastimil Babka (SUSE)
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=avd6XEHndh30hyc-@gremlin \
--to=ljs@kernel.org \
--cc=akpm@linux-foundation.org \
--cc=david@kernel.org \
--cc=jannh@google.com \
--cc=liam@infradead.org \
--cc=linux-kernel@vger.kernel.org \
--cc=linux-kselftest@vger.kernel.org \
--cc=linux-mm@kvack.org \
--cc=mhocko@suse.com \
--cc=rppt@kernel.org \
--cc=shuah@kernel.org \
--cc=stable@vger.kernel.org \
--cc=surenb@google.com \
--cc=tz2294@columbia.edu \
--cc=vbabka@kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.