Re: [PATCH v5 02/18] mm/huge_memory: preserve pmd_swp_uffd_wp on device-private PMD downgrade

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] The patch addresses uffd-wp flag preservation for device-private PMDs during permission downgrades, but leaves the exact same vulnerability unaddressed for migration entries across PMD, PTE, and hugetlb paths, leading to silent UFFD-WP bypass.
--

commit 053193c00433ae069a54ed063476afb79e232be6
Author: Kiryl Shutsemau (Meta) <kas@kernel.org>

mm/huge_memory: preserve pmd_swp_uffd_wp on device-private PMD downgrade

This patch fixes an issue where userfaultfd write-protection bits were being
dropped when downgrading the permissions of device-private PMD swap entries.
It modifies change_non_present_huge_pmd() to carry over the pmd_swp_uffd_wp
bit during this transition.

> change_non_present_huge_pmd() rewrites a writable device-private PMD
> swap entry into a readable one without carrying pmd_swp_uffd_wp()
> across. The PTE-level change_softleaf_pte() does this correctly;

[Severity: High]
This isn't a bug introduced by this patch, but is this statement entirely
accurate for migration entries? It appears change_softleaf_pte() only
preserves the soft_dirty bit for migration entries, not the uffd_wp bit.

> diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> index 42b86e8ab7c03..b7c895b1d3664 100644
> --- a/mm/huge_memory.c
> +++ b/mm/huge_memory.c
> @@ -2663,6 +2663,8 @@ static void change_non_present_huge_pmd(struct mm_struct *mm,

[ ... ]

>  	if (softleaf_is_migration_write(entry)) {
>  		/*
>  		 * A protection check is difficult so
>  		 * just be safe and disable write
>  		 */
>  		if (folio_test_anon(folio))
>  			entry = make_readable_exclusive_migration_entry(swp_offset(entry));
>  		else
>  			entry = make_readable_migration_entry(swp_offset(entry));
>  		newpmd = swp_entry_to_pmd(entry);
>  		if (pmd_swp_soft_dirty(*pmd))
>  			newpmd = pmd_swp_mksoft_dirty(newpmd);

[Severity: High]
This is a pre-existing issue, but does this migration entry path also need to
preserve the uffd_wp bit?

When mprotect(PROT_READ) downgrades writable migration entries to readable,
the uffd_wp bit is lost because swp_entry_to_pmd() discards it, and only
soft_dirty is explicitly restored here.

If a page is migrated concurrently with mprotect(), could this result in
userfaultfd traps being silently bypassed when the migration completes and
remove_migration_pte() restores the page without the uffd_wp bit?

>  	} else if (softleaf_is_device_private_write(entry)) {
>  		entry = make_readable_device_private_entry(swp_offset(entry));
>  		newpmd = swp_entry_to_pmd(entry);
> +		if (pmd_swp_uffd_wp(*pmd))
> +			newpmd = pmd_swp_mkuffd_wp(newpmd);
>  	} else {
>  		newpmd = *pmd;
>  	}

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260526130509.2748441-1-kirill@shutemov.name?part=2