Re: [PATCH] mm/madvise: disallow guard region installation in uffd-armed VMAs

["David Hildenbrand (Arm)" <david@kernel.org>]

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 ...

-- 
Cheers,

David