Re: [PATCH v2 5/9] mm/mprotect: keep writable entries writable on permission upgrades

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

On 11/5/26 17:50, Carlos López wrote:
> change_protection() applies vma->vm_page_prot, which lacks the write bit
> for CoW and write-notified mappings, so it write-protects every entry
> in them. When an entry would simply be made writable on fault without
> further checks (e.g. dirty PTEs in shared mappings, exclusive anonymous
> pages), MM_CP_TRY_CHANGE_WRITABLE makes the entry writable again to
> avoid the future fault.
> 
> However, writable entries in shared mappings that are not dirty still
> become read-only. For example, a uffd-wp unprotect makes clean shmem
> PTEs read-only even if they were never write-protected, e.g. when they
> were mapped by a read fault before uffd-wp was registered. The next
> write to each of them causes a fault that only restores the write bit.
> 
> Now that MM_CP_PERM_UPGRADE indicates whether mprotect() only adds
> permissions, change_protection() can tell, together with the
> userfaultfd unprotect flags, when a change revokes no access. Keep
> writable entries writable in these cases, at the PTE, PMD and PUD
> levels. This is consistent with the write fault handler, as an entry
> that is already writable needs no write fault.
> 
> Signed-off-by: Carlos López <clopez@suse.de>
> ---
>  mm/huge_memory.c | 7 +++++++
>  mm/internal.h    | 7 +++++++
>  mm/mprotect.c    | 4 ++++
>  3 files changed, 18 insertions(+)
> 
> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> index 506e1e61c0fd..6e23d0e4ae66 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -2809,6 +2809,10 @@ int change_huge_pmd(struct mmu_gather *tlb, struct vm_area_struct *vma,
>  	if (userfaultfd_rwp(vma) && pmd_uffd(entry))
>  		entry = pmd_modify(entry, PAGE_NONE);
>  
> +	/* See change_present_ptes(). */
> +	if (mm_cp_is_perm_upgrade(cp_flags) && pmd_write(oldpmd))
> +		entry = pmd_mkwrite(entry, vma);
> +
>  	/* See change_pte_range(). */
>  	if ((cp_flags & MM_CP_TRY_CHANGE_WRITABLE) && !pmd_write(entry) &&
>  	    can_change_pmd_writable(vma, addr, entry))
> @@ -2885,6 +2889,9 @@ int change_huge_pud(struct mmu_gather *tlb, struct vm_area_struct *vma,
>  	 */
>  	oldpud = pudp_invalidate(vma, addr, pudp);
>  	entry = pud_modify(oldpud, newprot);
> +	/* See change_present_ptes(). */
> +	if (mm_cp_is_perm_upgrade(cp_flags) && pud_write(oldpud))
> +		entry = pud_mkwrite(entry);

That looks completely hacked on. We do have can_change_pud_writable() below to
make such decisions.

Whaatever you propose here is either wrong or way too shaky, sorry.

-- 
Cheers,

David