linux-kernel - Re: [PATCH] mm: page_frag: Fix refill handling in __page_frag_alloc

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <cc3034c6-2589-4e9a-97af-a7879998d7d8@gmail.com>
Date: Sat, 1 Mar 2025 21:49:55 +0800
From: Yunsheng Lin <yunshenglin0825@...il.com>
To: Haiyang Zhang <haiyangz@...rosoft.com>, linux-hyperv@...r.kernel.org,
 akpm@...ux-foundation.org, linux-mm@...ck.org
Cc: decui@...rosoft.com, kys@...rosoft.com, paulros@...rosoft.com,
 olaf@...fle.de, vkuznets@...hat.com, davem@...emloft.net,
 wei.liu@...nel.org, longli@...rosoft.com, linux-kernel@...r.kernel.org,
 linyunsheng@...wei.com, stable@...r.kernel.org, netdev@...r.kernel.org,
 Alexander Duyck <alexander.duyck@...il.com>
Subject: Re: [PATCH] mm: page_frag: Fix refill handling in
 __page_frag_alloc_align()

+cc netdev ML & Alexander

On 3/1/2025 10:03 AM, Haiyang Zhang wrote:
> In commit 8218f62c9c9b ("mm: page_frag: use initial zero offset for
> page_frag_alloc_align()"), the check for fragsz is moved earlier.
> So when the cache is used up, and if the fragsz > PAGE_SIZE, it won't
> try to refill, and just return NULL.
> I tested it with fragsz:8192, cache-size:32768. After the initial four
> successful allocations, it failed, even there is plenty of free memory
> in the system.

Hi, Haiyang
It seems the PAGE_SIZE is 4K for the tested system?
Which drivers or subsystems are passing the fragsz being bigger than
PAGE_SIZE to page_frag_alloc_align() related API?

> To fix, revert the refill logic like before: the refill is attempted
> before the check & return NULL.

page_frag API is not really for allocating memory being bigger than
PAGE_SIZE as __page_frag_cache_refill() will not try hard enough to
allocate order 3 compound page when calling __alloc_pages() and will
fail back to allocate base page as the discussed in below:
https://lore.kernel.org/all/ead00fb7-8538-45b3-8322-8a41386e7381@huawei.com/

> 
> Cc: linyunsheng@...wei.com
> Cc: stable@...r.kernel.org
> Fixes: 8218f62c9c9b ("mm: page_frag: use initial zero offset for page_frag_alloc_align()")
> Signed-off-by: Haiyang Zhang <haiyangz@...rosoft.com>
> ---
>   mm/page_frag_cache.c | 26 +++++++++++++-------------
>   1 file changed, 13 insertions(+), 13 deletions(-)
> 
> diff --git a/mm/page_frag_cache.c b/mm/page_frag_cache.c
> index d2423f30577e..82935d7e53de 100644
> --- a/mm/page_frag_cache.c
> +++ b/mm/page_frag_cache.c
> @@ -119,19 +119,6 @@ void *__page_frag_alloc_align(struct page_frag_cache *nc,
>   	size = PAGE_SIZE << encoded_page_decode_order(encoded_page);
>   	offset = __ALIGN_KERNEL_MASK(nc->offset, ~align_mask);
>   	if (unlikely(offset + fragsz > size)) {
> -		if (unlikely(fragsz > PAGE_SIZE)) {
> -			/*
> -			 * The caller is trying to allocate a fragment
> -			 * with fragsz > PAGE_SIZE but the cache isn't big
> -			 * enough to satisfy the request, this may
> -			 * happen in low memory conditions.
> -			 * We don't release the cache page because
> -			 * it could make memory pressure worse
> -			 * so we simply return NULL here.
> -			 */
> -			return NULL;
> -		}
> -
>   		page = encoded_page_decode_page(encoded_page);
>   
>   		if (!page_ref_sub_and_test(page, nc->pagecnt_bias))
> @@ -149,6 +136,19 @@ void *__page_frag_alloc_align(struct page_frag_cache *nc,
>   		/* reset page count bias and offset to start of new frag */
>   		nc->pagecnt_bias = PAGE_FRAG_CACHE_MAX_SIZE + 1;
>   		offset = 0;
> +
> +		if (unlikely(fragsz > size)) {
> +			/*
> +			 * The caller is trying to allocate a fragment
> +			 * with fragsz > size but the cache isn't big
> +			 * enough to satisfy the request, this may
> +			 * happen in low memory conditions.
> +			 * We don't release the cache page because
> +			 * it could make memory pressure worse
> +			 * so we simply return NULL here.
> +			 */
> +			return NULL;
> +		}
>   	}
>   
>   	nc->pagecnt_bias--;