linux-kernel - Re: [PATCH 1/2] mm: page_frag: Check fragsz at the beginning of __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: <d06a75d7-c503-4aa1-a846-d23ef5c48d3c@huawei.com>
Date: Wed, 16 Apr 2025 15:37:38 +0800
From: Yunsheng Lin <linyunsheng@...wei.com>
To: Haiyang Zhang <haiyangz@...rosoft.com>, <linux-hyperv@...r.kernel.org>,
	<akpm@...ux-foundation.org>, <corbet@....net>, <linux-mm@...ck.org>,
	<linux-doc@...r.kernel.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>
Subject: Re: [PATCH 1/2] mm: page_frag: Check fragsz at the beginning of
 __page_frag_alloc_align()

On 2025/4/4 5:21, Haiyang Zhang wrote:
> Frag allocator is not designed for fragsz > PAGE_SIZE. So, check and return
> the error at the beginning of __page_frag_alloc_align(), instead of
> succeed for a few times, then fail due to not refilling the cache.
> 
> Signed-off-by: Haiyang Zhang <haiyangz@...rosoft.com>
> ---
>  mm/page_frag_cache.c | 22 +++++++++-------------
>  1 file changed, 9 insertions(+), 13 deletions(-)
> 
> diff --git a/mm/page_frag_cache.c b/mm/page_frag_cache.c
> index d2423f30577e..d6bf022087e7 100644
> --- a/mm/page_frag_cache.c
> +++ b/mm/page_frag_cache.c
> @@ -98,6 +98,15 @@ void *__page_frag_alloc_align(struct page_frag_cache *nc,
>  	unsigned int size, offset;
>  	struct page *page;
>  
> +	if (unlikely(fragsz > PAGE_SIZE)) {
> +		/*
> +		 * The caller is trying to allocate a fragment
> +		 * with fragsz > PAGE_SIZE which is not supported
> +		 * by design. So we simply return NULL here.
> +		 */
> +		return NULL;
> +	}

The checking is done at below to avoid doing the checking for the
likely case of cache being enough as the frag API is mostly used
to allocate small memory.

And it seems my recent refactoring to frag API have made two
frag API misuse more obvious if I recalled it correctly. If more
explicit about that for all the codepath is really helpful, perhaps
VM_BUG_ON() is an option to make it more explicit while avoiding
the checking as much as possible.

> +
>  	if (unlikely(!encoded_page)) {
>  refill:
>  		page = __page_frag_cache_refill(nc, gfp_mask);
> @@ -119,19 +128,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))