Re: [PATCH v2] mm/ksm: keep soft-dirty and uffd-wp when merging a page

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] mm/ksm: installing PAGE_NONE in replace_page() breaks break_ksm() on RWP PTEs
- [Medium] mm/ksm: replace_page() sets soft-dirty on clean KSM zero-page PTEs
--

commit e9f8ba7271894cc9b005a46048d0bcc61cff0fff
Author: Samarth Narang <samarth.colleges@gmail.com>

mm/ksm: keep soft-dirty and uffd-wp when merging a page

This commit preserves the soft-dirty and userfaultfd bits when replace_page()
installs a KSM PTE in place of an anonymous page. For VMAs registered with
userfaultfd RWP, it also modifies the new PTE with PAGE_NONE when the uffd bit
is set.

> diff --git a/mm/ksm.c b/mm/ksm.c
> index dbfb8f98b1892..de144fa07c493 100644
> --- a/mm/ksm.c
> +++ b/mm/ksm.c
[ ... ]
> @@ -1463,6 +1464,17 @@ static int replace_page(struct vm_area_struct *vma, struct page *page,
>  		dec_mm_counter(mm, MM_ANONPAGES);
>  	}
>  
> +	/*
> +	 * Preserve soft-dirty and uffd bits when replacing the page.
> +	 * Restore PAGE_NONE for RWP-protected PTEs.
> +	 */
> +	if (pte_soft_dirty(orig_pte))
> +		newpte = pte_mksoft_dirty(newpte);

[Severity: Medium]
When ksm_use_zero_pages is enabled and replace_page() merges a zero-filled
page whose soft-dirty bit was cleared, earlier in replace_page() the zero-page
PTE is constructed with pte_mkdirty():

    newpte = pte_mkdirty(pte_mkspecial(pfn_pte(page_to_pfn(kpage), vma->vm_page_prot)));

On architectures such as x86, pte_mkdirty() sets both _PAGE_DIRTY and
_PAGE_SOFT_DIRTY:

arch/x86/include/asm/pgtable.h:pte_mkdirty() {
	pte = pte_set_flags(pte, _PAGE_DIRTY | _PAGE_SOFT_DIRTY);

	return pte_mksaveddirty(pte);
}

Without an else branch calling pte_clear_soft_dirty(newpte) (similar to
remove_migration_pte() in mm/migrate.c), won't clean pages merged with the KSM
zero page end up falsely marked soft-dirty in /proc/<pid>/pagemap?

> +	if (pte_uffd(orig_pte))
> +		newpte = pte_mkuffd(newpte);
> +	if (pte_uffd(orig_pte) && userfaultfd_rwp(vma))
> +		newpte = pte_modify(newpte, PAGE_NONE);

[Severity: High]
How does installing a KSM PTE with PAGE_NONE and the uffd bit interact with
break_ksm() when unmerging KSM pages (via ksm_madvise() for MADV_UNMERGEABLE,
prep_move_vma() during mremap(), __ksm_del_vma(),
unmerge_and_remove_all_rmap_items(), or break_cow())?

break_ksm() invokes handle_mm_fault() with FAULT_FLAG_UNSHARE |
FAULT_FLAG_REMOTE (and without FAULT_FLAG_ALLOW_RETRY):

mm/ksm.c:break_ksm() {
	do {
		...
		ret = handle_mm_fault(vma, addr,
				      FAULT_FLAG_UNSHARE | FAULT_FLAG_REMOTE,
				      NULL);
	} while (!(ret & (VM_FAULT_SIGBUS | VM_FAULT_SIGSEGV | VM_FAULT_OOM)));
}

In handle_pte_fault(), the pte_protnone() check runs before the
FAULT_FLAG_UNSHARE check and dispatches to do_uffd_rwp():

mm/memory.c:handle_pte_fault() {
	if (pte_protnone(vmf->orig_pte) && vma_is_accessible(vmf->vma)) {
		if (userfaultfd_pte_rwp(vmf->vma, vmf->orig_pte))
			return do_uffd_rwp(vmf);
		return do_numa_page(vmf);
	}
}

1. In synchronous RWP mode (!userfaultfd_rwp_async(vmf->vma)), do_uffd_rwp()
calls handle_userfault(vmf, VM_UFFD_RWP). Because FAULT_FLAG_ALLOW_RETRY is
not set, handle_userfault() triggers the CONFIG_DEBUG_VM warning for missing
FAULT_FLAG_ALLOW_RETRY and returns VM_FAULT_SIGBUS. break_ksm() then exits its
loop and returns 0, leaving the KSM page (and any subsequent KSM pages in the
range) still mapped after MADV_UNMERGEABLE, mremap(), or VMA teardown.

2. In asynchronous RWP mode (userfaultfd_rwp_async(vmf->vma)), the first
iteration of break_ksm() runs do_uffd_rwp(), which restores vma->vm_page_prot,
clears the uffd bit, and marks the PTE young, and the second iteration
unshares the page via do_wp_page() -> wp_page_copy(), silently stripping RWP
protection during an internal KSM unmerge.

3. Even if handle_pte_fault() were to reach wp_page_copy() directly for
FAULT_FLAG_UNSHARE, wp_page_copy() copies pte_uffd(vmf->orig_pte) onto entry
built from vma->vm_page_prot without restoring PAGE_NONE when
userfaultfd_rwp(vma) is true:

mm/memory.c:wp_page_copy() {
		if (unlikely(unshare)) {
			if (pte_soft_dirty(vmf->orig_pte))
				entry = pte_mksoft_dirty(entry);
			if (pte_uffd(vmf->orig_pte))
				entry = pte_mkuffd(entry);
		}
}

Does FAULT_FLAG_UNSHARE in handle_pte_fault() and wp_page_copy() need to
handle RWP-protected PTEs before KSM merges them with PAGE_NONE?

>  
>  	flush_cache_page(vma, addr, pte_pfn(ptep_get(ptep)));
>  	/*

[]

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261019010934.697895-1-samarth.colleges@gmail.com?part=1