Re: [PATCH mm-hotfixes] mm/huge_memory: use correct flags for device private PMD entry

[Lorenzo Stoakes]

On Tue, Jun 02, 2026 at 10:40:16AM -0400, Zi Yan wrote:
> On 1 Jun 2026, at 4:30, Lorenzo Stoakes wrote:
>
> > Commit 65edfda6f3f2 ("mm/rmap: extend rmap and migration support
> > device-private entries") updated set_pmd_migration_entry() to use
> > pmdp_huge_get_and_clear() in the softleaf case, but made no further
> > adjustments to the function itself.
> >
> > Therefore this function continues to incorrectly use pmd_write(),
> > pmd_soft_dirty() and pmd_uffd_wp() to determine whether the installed
> > migration entry should be marked writable, softdirty or uffd-wp
> > respectively.
> >
> > Whilst all are incorrect, the most problematic of these is pmd_write(), as
> > this can lead to corrupted rmap state.
> >
> > On x86-64 _PAGE_SWP_SOFT_DIRTY is aliased to _PAGE_RW. So calling
> > pmd_write() on a softleaf will return the softdirty state encoded in the
> > entry, assuming CONFIG_MEM_SOFT_DIRTY was enabled.
> >
> > This was observed when running the hmm.hmm_device_private.anon_write_child
> > selftest:
> >
> > 1. The test faults in a range then migrates it such that a device-private
> >    THP range is established.
> >
> > 2. The parent then migrates it to a device-private writable PMD entry whose
> >    folio is entirely AnonExclusive with entire_mapcount=1, softdirty set
> >    (accidentally correct write state).
> >
> > 3. The parent forks and the PMD entries are set to device-private read only
> >    entries, entire_mapcount=2, softdirty still set.
> >
> > 4. [BUG] The child writes to the range then migrates to RAM - intending to
> >    install non-writable migration entries - but replacing parent and child
> >    PMD mappings with WRITABLE entries due to misinterpreting the softdirty
> >    bit.
> >
> > 5. In remove_migration_pmd(), if !softleaf_is_migration_read(entry) we
> >    set the RMAP_EXCLUSIVE flag when calling folio_add_anon_rmap_pmd() for
> >    both parent and child, which are therefore AnonExclusive.
> >
> > 6. [SPLAT] Child sets migrated folio entire_mapcount=1, parent sets
> >    entire_mapcount=2 and we end up with an AnonExclusive folio with
> >    entire_mapcount=2! Assert fires in __folio_add_anon_rmap():
> >
> > 		VM_WARN_ON_FOLIO(folio_test_large(folio) &&
> > 				 folio_entire_mapcount(folio) > 1 &&
> > 				 PageAnonExclusive(cur_page), folio)
> >
> > This patch fixes the issue by correctly referencing the softleaf entry
> > fields for writable, softdirty and uffd-wp in set_pmd_migration_entry().
> >
> > It also only updates A/D flags if the entry is present as these are
> > otherwise not meaningful for a softleaf entry.
> >
> > This patch also flips the if (!present) { ... } else { ... } logic in
> > set_pmd_migration_entry() so it is easier to understand, and adds some
> > comments to make things clearer.
> >
> > I was able to bisect this to commit 775465fd26a3 ("lib/test_hmm: add zone
> > device private THP test infrastructure") which first exposes this bug as it
> > was the commit that permitted test_hmm to generate the test.
> >
> > However commit 65edfda6f3f2 ("mm/rmap: extend rmap and migration support
> > device-private entries") is the commit that actually enabled this
> > behaviour.
>
> Thanks for the detailed explanation.
> >
> > Fixes: 65edfda6f3f2 ("mm/rmap: extend rmap and migration support device-private entries")
> > Cc: stable@xxxxxxxxxxxxxxx
> > Signed-off-by: Lorenzo Stoakes <ljs@xxxxxxxxxx>
> > ---
> >  mm/huge_memory.c | 45 +++++++++++++++++++++++++++++++++------------
> >  1 file changed, 33 insertions(+), 12 deletions(-)
> >
> > diff --git a/mm/huge_memory.c b/mm/huge_memory.c
> > index bf9b480bb3b0..79463c709c98 100644
> > --- a/mm/huge_memory.c
> > +++ b/mm/huge_memory.c
> > @@ -4982,7 +4982,7 @@ int set_pmd_migration_entry(struct page_vma_mapped_walk *pvmw,
> >  	struct vm_area_struct *vma = pvmw->vma;
> >  	struct mm_struct *mm = vma->vm_mm;
> >  	unsigned long address = pvmw->address;
> > -	bool anon_exclusive;
> > +	bool anon_exclusive, present, writable, softdirty, uffd_wp;
> >  	pmd_t pmdval;
> >  	swp_entry_t entry;
> >  	pmd_t pmdswp;
> > @@ -4990,12 +4990,26 @@ int set_pmd_migration_entry(struct page_vma_mapped_walk *pvmw,
> >  	if (!(pvmw->pmd && !pvmw->pte))
> >  		return 0;
> >
> > -	flush_cache_range(vma, address, address + HPAGE_PMD_SIZE);
> > -	if (unlikely(!pmd_present(*pvmw->pmd)))
> > -		pmdval = pmdp_huge_get_and_clear(vma->vm_mm, address, pvmw->pmd);
> > -	else
> > +	present = pmd_present(*pvmw->pmd);
> > +	if (likely(present)) {
> > +		flush_cache_range(vma, address, address + HPAGE_PMD_SIZE);
> > +
> >  		pmdval = pmdp_invalidate(vma, address, pvmw->pmd);
> >
> > +		writable = pmd_write(pmdval);
> > +		softdirty = pmd_soft_dirty(pmdval);
> > +		uffd_wp = pmd_uffd_wp(pmdval);
> > +	} else {
> > +		softleaf_t old_entry;
> > +
> > +		pmdval = pmdp_huge_get_and_clear(vma->vm_mm, address, pvmw->pmd);
> > +		old_entry = softleaf_from_pmd(pmdval);
> > +
> > +		writable = softleaf_is_device_private_write(old_entry);
>
> Just to make sure I get it. This means the only possible writable
> non present/softleaf entry is device private writable. There is
> writable migration entry, but since we are setting a migration entry
> here, that should not be possible.

Yes :)

This is doing the same as try_to_migrate_one(), e.g.:

		if (folio_test_hugetlb(folio)) {
			...
		} else if (likely(pte_present(pteval))) {
			...
		} else {
			const softleaf_t entry = softleaf_from_pte(pteval);

			pte_clear(mm, address, pvmw.pte);

			writable = softleaf_is_device_private_write(entry);
		}

>
> The patch LGTM. Thanks.
>
> Reviewed-by: Zi Yan <ziy@xxxxxxxxxx>

Thanks!

>
>
> Best Regards,
> Yan, Zi

Cheers, Lorenzo