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

[Muchun Song <muchun.song@linux.dev>]

[-- Attachment #1: Type: text/plain, Size: 4335 bytes --]



> On Nov 12, 2026, at 10:59, Muchun Song <muchun.song@linux.dev> wrote:
> 
> 
> 
>> On Nov 12, 2026, at 06:38, sashiko-bot@kernel.org wrote:
>> 
>> 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?
> 
> I think the following diff will fix this.
> 
> diff --git a/mm/memory-failure.c b/mm/memory-failure.c
> index 164a02866fd5..abe92db14fbc 100644
> --- a/mm/memory-failure.c
> +++ b/mm/memory-failure.c
> @@ -2814,6 +2814,7 @@ int unpoison_memory(unsigned long pfn)
> 
>        ghp = get_hwpoison_page(p, MF_UNPOISON);
>        if (!ghp) {
> +               guard(vmemmap_remap)();
>                if (folio_test_hugetlb(folio)) {
>                        huge = true;
>                        count = folio_free_raw_hwp(folio, false);
> @@ -2830,6 +2831,7 @@ int unpoison_memory(unsigned long pfn)
>                                         pfn, &unpoison_rs);
>                }
>        } else {
> +               guard(vmemmap_remap)();

Sorry. This is unnecessary since we already hold a refcount to the folio.

>                if (folio_test_hugetlb(folio)) {
>                        huge = true;
>                        count = folio_free_raw_hwp(folio, false);
> 
>> 
>> 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


[-- Attachment #2: Type: text/html, Size: 26775 bytes --]