Re: [PATCH] mm/hugetlb: clear the hugetlb flag before leaking a hwpoisoned folio

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Critical] mm/hugetlb: add_hugetlb_folio() clears HPG_raw_hwp_unreliable on error
--

commit cae6d1104a254ddb073fab05f2a9308005af88fa
Author: Mani Tofigh <mt4022@columbia.edu>

mm/hugetlb: clear the hugetlb flag before leaking a hwpoisoned folio

This commit moves the folio_test_hugetlb_raw_hwp_unreliable() check in
__update_and_free_hugetlb_folio() to after the hugetlb flag is cleared, so
that vmemmap-optimized folios with unreliable hwpoison tracking are leaked as
non-hugetlb folios instead of causing dissolve paths to loop indefinitely.

> diff --git a/mm/hugetlb.c b/mm/hugetlb.c
> index 4a008afaedf95..4d6b7ab49502f 100644
> --- a/mm/hugetlb.c
> +++ b/mm/hugetlb.c
> @@ -1472,13 +1472,6 @@ static void __update_and_free_hugetlb_folio(struct hstate *h,
>  		return;
>  	}
>  
> -	/*
> -	 * If we don't know which subpages are hwpoisoned, we can't free
> -	 * the hugepage, so it's leaked intentionally.
> -	 */
> -	if (folio_test_hugetlb_raw_hwp_unreliable(folio))
> -		return;
> -
>  	/*
>  	 * If folio is not vmemmap optimized (!clear_flag), then the folio
>  	 * is no longer identified as a hugetlb page.  hugetlb_vmemmap_restore_folio
> @@ -1506,6 +1499,14 @@ static void __update_and_free_hugetlb_folio(struct hstate *h,
>  		spin_unlock_irq(&hugetlb_lock);
>  	}
>  
> +	/*
> +	 * If we don't know which subpages are hwpoisoned, we can't free
> +	 * the hugepage, so it's leaked intentionally, but only once it is
> +	 * no longer a hugetlb folio.
> +	 */
> +	if (folio_test_hugetlb_raw_hwp_unreliable(folio))
> +		return;

[Severity: Critical]
Can moving folio_test_hugetlb_raw_hwp_unreliable() below
hugetlb_vmemmap_restore_folio() cause hardware-poisoned subpages to be freed
back to the buddy allocator as healthy pages if vmemmap restoration fails?

When hugetlb_update_hwpoison() fails to allocate a raw_hwp_page struct, it
marks the folio with HPG_raw_hwp_unreliable and frees the raw_hwp_page list
without transferring PG_hwpoison to the subpages:

mm/memory-failure.c:hugetlb_update_hwpoison() {
	...
		folio_set_hugetlb_raw_hwp_unreliable(folio);
		/*
		 * Once hugetlb_raw_hwp_unreliable is set, raw_hwp_page is not
		 * used any more, so free it.
		 */
		__folio_free_raw_hwp(folio, false);
	...
}

With this change, __update_and_free_hugetlb_folio() now calls
hugetlb_vmemmap_restore_folio() on a raw_hwp_unreliable folio before checking
the flag. If hugetlb_vmemmap_restore_folio() fails to allocate vmemmap pages,
the error path puts the folio back on the hugetlb freelist via
add_hugetlb_folio():

mm/hugetlb.c:__update_and_free_hugetlb_folio() {
	...
	if (clear_flag && hugetlb_vmemmap_restore_folio(h, folio)) {
		spin_lock_irq(&hugetlb_lock);
		add_hugetlb_folio(h, folio, true);
		spin_unlock_irq(&hugetlb_lock);
		return;
	}
	...
}

However, add_hugetlb_folio() zeroes folio->private and only restores
HPG_vmemmap_optimized, which clears HPG_raw_hwp_unreliable (as well as
HPG_cma):

mm/hugetlb.c:add_hugetlb_folio() {
	...
	__folio_set_hugetlb(folio);
	/* Clear all folio->private flags except hugetlb_vmemmap_optimized. */
	folio->private = NULL;
	folio_set_hugetlb_vmemmap_optimized(folio);
	...
}

If the folio is later freed or dissolved and hugetlb_vmemmap_restore_folio()
succeeds, folio_test_hugetlb_raw_hwp_unreliable(folio) will now return false.
Wouldn't __update_and_free_hugetlb_folio() then call
folio_clear_hugetlb_hwpoison(), clearing PG_hwpoison on the head page against
an empty raw_hwp_page list, and release the poisoned subpages to
free_frozen_pages() as clean pages?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261028-hugetlb-hwpoison-leak-v1-1-77ce56002523@columbia.edu?part=1