Re: [PATCH v2] mm/page_alloc: more checks for alloc_contig_frozen_range() with __GFP_COMP

[David Hildenbrand (Arm)]

On 11/2/26 17:24, Zi Yan wrote:
> When alloc_contig_frozen_range() is called with __GFP_COMP, certain inputs
> can cause a mismatch between a requested range and its allocated range,
> leading to the WARN at the end of the function and page leaks. No in-tree
> user passes such inputs. Add checks to prevent it:
> 
> 1. the order is at least MAX_PAGE_ORDER: free pages during isolation can
>    merge up to MAX_PAGE_ORDER, extending the allocated range beyond the
>    requested range;
> 2. the number of pages in the range is a power of 2: ilog2() can round
>    down, so that the compound page will not cover the requested range;
> 3. start and end are naturally aligned to nr_pages: a compound page must be
>    naturally aligned, and the unaligned range can also lead to a mismatched
>    allocated range.
> 
> Because of check 2, the same power of 2 at the end is redundant and
> removed.
> 
> Add a comment on the WARN path to clarify it should not be reachable, so no
> one in the future will try to fix the impossible page leak.
> 
> Reported-by: Sashiko <sashiko-bot@xxxxxxxxxx>
> Closes: https://sashiko.dev/#/patchset/20260628-keep-subpage-private-zero-at-free-v1-0-f4ce3930d10f%40nvidia.com?part=1
> Signed-off-by: Zi Yan <ziy@xxxxxxxxxx>
> ---
> Sashiko reports that if alloc_contig_frozen_range() with __GFP_COMP can
> allocate a free page range different from the requested range, the function
> triggers a WARN and returns EINVAL without freeing the allocated PFNs and
> causes free memory leaks. Add checks to prevent it.
> 
> The is_power_of_2() check and start PFN alignment check are added with the
> help of LLM.
> ---
> Changes in v2:
> 1. added 3 checks for __GFP_COMP to prevent the allocated range from
>    mismatching the requested range, which can trigger the warning.
> 2. removed the redundant power of 2 check at the end of the function.
> 3. added a comment on the WARN path to clarify it becomes unreachable.
> 4. dropped Fixes tag.
> - Link to v1: https://lore.kernel.org/r/20260629-free-pfn-on-alloc-contig-range-error-path-v1-1-496ff9ca22db@xxxxxxxxxx
> ---
>  mm/page_alloc.c | 25 +++++++++++++++++++------
>  1 file changed, 19 insertions(+), 6 deletions(-)
> 
> diff --git a/mm/page_alloc.c b/mm/page_alloc.c
> index 15ff5c1feef4f..003911ef393be 100644
> --- a/mm/page_alloc.c
> +++ b/mm/page_alloc.c
> @@ -7326,12 +7326,24 @@ int alloc_contig_frozen_range_noprof(unsigned long start, unsigned long end,
>  					    PB_ISOLATE_MODE_OTHER;
>  
>  	/*
> -	 * 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 use of __GFP_COMP has the following requirements:
> +	 * 1. the order cannot exceed MAX_FOLIO_ORDER (>MAX_PAGE_ORDER is OK);
> +	 * 2. the order is at least MAX_PAGE_ORDER to ensure at the end
> +	 *    outer_start and outer_end match start and end respectively;
> +	 * 3. the number of pages in the range is a power of 2;
> +	 * 4. start and end are naturally aligned to nr_pages.
> +	 *
> +	 * 3 and 4 are also the requirements for __GFP_COMP pages in general.
>  	 */
> -	if (WARN_ON_ONCE((gfp_mask & __GFP_COMP) && order > MAX_FOLIO_ORDER))
> -		return -EINVAL;
> +	if (gfp_mask & __GFP_COMP) {
> +		const unsigned long nr_pages = end - start;
> +
> +		if (WARN_ON_ONCE(order > MAX_FOLIO_ORDER ||
> +				 order < MAX_PAGE_ORDER ||
> +				 !is_power_of_2(nr_pages) ||
> +				 !IS_ALIGNED(start, nr_pages)))
> +			return -EINVAL;
> +	}
>  
>  	gfp_mask = current_gfp_context(gfp_mask);
>  	if (__alloc_contig_verify_gfp_mask(gfp_mask, (gfp_t *)&cc.gfp_mask))
> @@ -7428,12 +7440,13 @@ int alloc_contig_frozen_range_noprof(unsigned long start, unsigned long end,
>  			__free_contig_frozen_range(outer_start, start - outer_start);
>  		if (end != outer_end)
>  			__free_contig_frozen_range(end, outer_end - end);
> -	} else if (start == outer_start && end == outer_end && is_power_of_2(end - start)) {
> +	} else if (start == outer_start && end == outer_end) {
>  		struct page *head = pfn_to_page(start);
>  
>  		check_new_pages(head, order);
>  		prep_new_page(head, order, gfp_mask, ALLOC_DEFAULT);
>  	} else {
> +		/* Unreachable with the __GFP_COMP checks above */
>  		ret = -EINVAL;
>  		WARN(true, "PFN range: requested [%lu, %lu), allocated [%lu, %lu)\n",
>  		     start, end, outer_start, outer_end);
> 
> ---
> base-commit: afb4eea28037fa55c4560313ecbd8a1e1da8369a
> change-id: 20260629-free-pfn-on-alloc-contig-range-error-path-acc001232468
> 
> Best regards,
> --  
> Yan, Zi
> 

Thanks!

Reviewed-by: David Hildenbrand (Arm) <david@xxxxxxxxxx>

-- 
Cheers,

David