Re: [PATCH] userfaultfd: mark uffd_wp regardless of VM_WRITE flag

[Peter Xu <peterx@redhat.com>]

Hello, Nadav,

On Thu, Feb 17, 2022 at 09:16:02PM +0000, Nadav Amit wrote:
> From: Nadav Amit <namit@vmware.com>
> 
> When a PTE is set by UFFD operations such as UFFDIO_COPY, the PTE is
> currently only marked as write-protected if the VMA has VM_WRITE flag
> set. This seems incorrect or at least would be unexpected by the users.
> 
> Consider the following sequence of operations that are being performed
> on a certain page:
> 
> 	mprotect(PROT_READ)
> 	UFFDIO_COPY(UFFDIO_COPY_MODE_WP)
> 	mprotect(PROT_READ|PROT_WRITE)

No objection to the patch, however I'm wondering why this is a valid use
case because mprotect seems to be conflict with uffd, because AFAICT
mprotect(PROT_READ|PROT_WRITE) can already grant write bit.

In change_pte_range():

        if (dirty_accountable && pte_dirty(ptent) &&
                        (pte_soft_dirty(ptent) ||
                                !(vma->vm_flags & VM_SOFTDIRTY))) {
                ptent = pte_mkwrite(ptent);
        }

PS: I always think here the VM_SOFTDIRTY check is wrong, IMHO it should be:

        if (dirty_accountable && pte_dirty(ptent) &&
                        (pte_soft_dirty(ptent) ||
                        (vma->vm_flags & VM_SOFTDIRTY))) {
                ptent = pte_mkwrite(ptent);
        }

Because when VM_SOFTDIRTY is cleared it means soft dirty enabled.  I wanted
to post a patch but I never yet.

Could I ask why you need mprotect() with uffd?

Thanks,

-- 
Peter Xu