Re: [PATCH] mm/page_alloc: free allocated PFNs if the range does not match

[Zi Yan]

On Tue Jun 30, 2026 at 9:39 AM EDT, David Hildenbrand (Arm) wrote:
> On 6/30/26 09:44, Vlastimil Babka (SUSE) wrote:
>> On 6/30/26 03:35, Zi Yan wrote:
>>> When using __GFP_COMP in alloc_contig_frozen_range(), if the allocated
>>> range does not match the requested one, the code errors out with EINVAL
>>> without freeing the allocated PFNs and causes free page leaks. Fix it by
>>> calling release_free_list() in the error path.
>>>
>>> The issue is reported by Sashiko[1].
>> 
>> So this?
>> Reported-by: Sashiko <sashiko-bot@xxxxxxxxxx>
>> 
>>> Fixes: e98337d11bbd ("mm/contig_alloc: support __GFP_COMP")
>>> Link: https://sashiko.dev/#/patchset/20260628-keep-subpage-private-zero-at-free-v1-0-f4ce3930d10f@xxxxxxxxxx [1]
>>> Signed-off-by: Zi Yan <ziy@xxxxxxxxxx>
>>> Cc: stable@xxxxxxxxxxxxxxx
>> 
>> Hm well, it's a path that warns, can only happen due to a development error?
>> Not sure we care about stable then. Anyway.
>> 
>
> If someone would run into the WARN we would already be in Fixes: territory.
>
> it's a path that should never be executed. If it does, the real issue must be fixed.
>
> So (a) I don't think this is stable material (b) I am skeptical that this is
> even a Fixes and (c) I am wondering whether we should touch this *at all*.
>
> :)

I looked at the code again and agree with you that the code is not
reachable and the fix should not be in the WARN path. Theoretically, if
order = ilog2(end - start) is smaller than MAX_PAGE_ORDER,
find_large_buddy() can return an outer_start smaller than start, leading
to this WARN path. But currently alloc_contig_frozen_range() with
__GFP_COMP is used by gigantic hugetlb, thus that is not possible.

How about
1. making sure order is bigger or equal to MAX_PAGE_ORDER,
2. adding a comment in the WARN path to prevent someone else trying to
fix WARN path if Sashiko reports this again

like the patch below?

diff --git a/mm/page_alloc.c b/mm/page_alloc.c
index ee902a468c2f..e87d3fced9d4 100644
--- a/mm/page_alloc.c
+++ b/mm/page_alloc.c
@@ -7130,8 +7130,13 @@ int alloc_contig_frozen_range_noprof(unsigned long start, unsigned long end,
 	 * In contrast to the buddy, we allow for orders here that exceed
 	 * MAX_PAGE_ORDER, so we must manually make sure that we are not
 	 * exceeding the maximum folio order.
+	 *
+	 * The order cannot be smaller than MAX_PAGE_ORDER either to prevent a
+	 * potential mismatch between the requested range and the allocated
+	 * range that leads to an allocation failure.
 	 */
-	if (WARN_ON_ONCE((gfp_mask & __GFP_COMP) && order > MAX_FOLIO_ORDER))
+	if (WARN_ON_ONCE((gfp_mask & __GFP_COMP) &&
+			 (order > MAX_FOLIO_ORDER || order < MAX_PAGE_ORDER))
 		return -EINVAL;
 
 	gfp_mask = current_gfp_context(gfp_mask);
@@ -7235,6 +7240,7 @@ int alloc_contig_frozen_range_noprof(unsigned long start, unsigned long end,
 		check_new_pages(head, order);
 		prep_new_page(head, order, gfp_mask, 0);
 	} else {
+		/* Fix the caller if this is reachable */
 		ret = -EINVAL;
 		WARN(true, "PFN range: requested [%lu, %lu), allocated [%lu, %lu)\n",
 		     start, end, outer_start, outer_end);


-- 
Best Regards,
Yan, Zi