Re: [PATCH RFC] mm/userfaultfd: enable writenotify while userfaultfd-wp is enabled for a VMA
From: Hugh Dickins
Date: Tue Dec 06 2022 - 14:10:02 EST
On Tue, 6 Dec 2022, David Hildenbrand wrote:
...
>
> We never had to remove write permissions so far from the vma->vm_page_prot
> default. We always only added permissions.
>
>
> Now, uffd-wp on shmem as of now violates these semantics. vma->vm_page_prot
> cannot always be used as a safe default, because we might have to wrprotect
> individual PTEs. Note that for uffd-wp on anonymous memory this was not an
> issue, because we default to !write in vma->vm_page_prot.
>
>
> The two possible ways to approach this for uffd-wp on shmem are:
>
> (1) Obey existing vma->vm_page_prot semantics. Default to !write and
> optimize the relevant cases to *add* the write bit. This is
> essentially what this patch does, minus
> can_change_pte_writable() optimizations on relevant code paths where
> we'd want to maintain the write bit. For example, when removing
> uffd-wp protection we might want to restore the write bit directly.
>
> (2) Default to write permissions and check each and every code location
> where we remap for uffd-wp ptes, to manuall wrprotect -- *remove*
> the write bit. Alternatively, as you said, always wrprotect when
> setting the PTE bit, which might work as well.
>
>
> My claim is that (1) is less error prone, because in the worst case we forget
> to optimize one code location -- instead to resulting in a BUG if we forget to
> wrprotect (what we have now). But I am not going to fight for it, because I
> can see that (2) can be made to work as well, as you outline in your patch.
>
> You seem to have decided on (2). Fair enough, you know uffd-wp best. We just
> have to fix it properly and make the logic consistent whenever we remap a
> page.
>
...
>
> But I'm not going to argue about whats valid and whats not as long as it's
> documented ;). I primarily wanted to showcase that the same logic based on
> vma->vm_page_prot is used elsewhere, and that migration PTE restoration is not
> particularly special.
I have not been following the uffd-wp work, but I believe that David's
painstaking and excellent account of vm_page_prot is correct. Peter,
please I beg you to follow his advice and go for (1) for uffd-wp.
I do not share David's faith in "documented": documented or not,
depart from safe convention and you will be adding (at least the
opportunity for) serious bugs.
Hugh