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

[Baolin Wang]

On 6/1/26 4:30 PM, 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.
>
> Fixes: 65edfda6f3f2 ("mm/rmap: extend rmap and migration support device-private entries")
> Cc: stable@xxxxxxxxxxxxxxx
> Signed-off-by: Lorenzo Stoakes <ljs@xxxxxxxxxx>
> ---

Thanks for your detailed explanation. Feel free to add:
Reviewed-by: Baolin Wang <baolin.wang@xxxxxxxxxxxxxxxxx>