Re: [PATCH v2 1/3] mm/hugetlb_vmemmap: fix lost HWPoison state during optimization

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential
issue to consider:
- [Medium] mm/memory-failure: unserialized unpoison_memory() races with
  HVO head remap
--

commit c0c1b7e9976626bf636152889fd9f5d88944d78b
Author: Muchun Song <muchun.song@linux.dev>

mm/hugetlb_vmemmap: fix lost HWPoison state during optimization

This commit introduces vmemmap_remap_rwsem to serialize HVO head page
remapping against HWPoison state updates in get_huge_page_for_hwpoison().

> diff --git a/mm/memory-failure.c b/mm/memory-failure.c
> index 78f0a13349cb3..164a02866fd59 100644
> --- a/mm/memory-failure.c
> +++ b/mm/memory-failure.c

[ ... ]

> @@ -2038,6 +2039,7 @@ static int get_huge_page_for_hwpoison(unsigned long pfn, int flags,
>  	bool count_increased = false;
>  	int ret, rc;
>  
> +	guard(vmemmap_remap)();
>  	spin_lock_irq(&hugetlb_lock);
>  	folio = page_folio(page);
>  	if (!folio_test_hugetlb(folio)) {

[Severity: Medium]
Does unpoison_memory() also need vmemmap_remap_rwsem when clearing
PG_hwpoison and freeing folio->_hugetlb_hwpoison?

If a newly initialized hugetlb folio is poisoned by a software-simulated
memory failure before HVO acquires vmemmap_remap_rwsem, unpoison_memory()
can race with the head page remap:

CPU0                                CPU1
hugetlb_vmemmap_optimize_folio()
  down_read(&vmemmap_remap_rwsem)
  ...
    vmemmap_optimize_pte()
      vmemmap_dup_page_from_list(old_head, ...)
                                    unpoison_memory()
                                      get_hwpoison_page(..., MF_UNPOISON)
                                      folio_free_raw_hwp(old_head, false)
                                      folio_test_clear_hwpoison(old_head)
      set_pte_at(..., new_head)
      flush_tlb_all()
                                    /* new_head retains PG_hwpoison and
                                     * freed _hugetlb_hwpoison pointer */

In get_hwpoison_hugetlb_folio(), passing unpoison == true takes the
folio_try_get(folio) branch even when !folio_test_hugetlb_migratable(folio),
returning 0 on the frozen folio instead of -EBUSY:

mm/hugetlb.c:get_hwpoison_hugetlb_folio() {
    ...
		if (folio_test_hugetlb_freed(folio))
			ret = 0;
		else if (folio_test_hugetlb_migratable(folio) || unpoison)
			ret = folio_try_get(folio);
		else
			ret = -EBUSY;
    ...
}

unpoison_memory() then sees ghp == 0 and calls folio_free_raw_hwp(folio,
false), which skips the folio_test_hugetlb_vmemmap_optimized(folio) check
used when move_flag is true and frees the struct raw_hwp_page entries from
old_head.  Once CPU0 installs new_head and flushes the TLB, the folio still
has PG_hwpoison set and folio->_hugetlb_hwpoison pointing to freed memory,
leading to a use-after-free or double free when the list is later walked or
freed.

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261111093747.50048-1-songmuchun@bytedance.com?part=1