linux-kernel - RE: [EXTERNAL] 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] [day] [month] [year] [list]

Message-ID:
 <MN0PR21MB3437AFFB3A7205C994F5551CCABD2@MN0PR21MB3437.namprd21.prod.outlook.com>
Date: Wed, 16 Apr 2025 14:11:58 +0000
From: Haiyang Zhang <haiyangz@...rosoft.com>
To: Yunsheng Lin <linyunsheng@...wei.com>, "linux-hyperv@...r.kernel.org"
	<linux-hyperv@...r.kernel.org>, "akpm@...ux-foundation.org"
	<akpm@...ux-foundation.org>, "corbet@....net" <corbet@....net>,
	"linux-mm@...ck.org" <linux-mm@...ck.org>, "linux-doc@...r.kernel.org"
	<linux-doc@...r.kernel.org>
CC: Dexuan Cui <decui@...rosoft.com>, KY Srinivasan <kys@...rosoft.com>, Paul
 Rosswurm <paulros@...rosoft.com>, "olaf@...fle.de" <olaf@...fle.de>,
	"vkuznets@...hat.com" <vkuznets@...hat.com>, "davem@...emloft.net"
	<davem@...emloft.net>, "wei.liu@...nel.org" <wei.liu@...nel.org>, Long Li
	<longli@...rosoft.com>, "linux-kernel@...r.kernel.org"
	<linux-kernel@...r.kernel.org>
Subject: RE: [EXTERNAL] Re: [PATCH 1/2] mm: page_frag: Check fragsz at the
 beginning of __page_frag_alloc_align()



> -----Original Message-----
> From: Yunsheng Lin <linyunsheng@...wei.com>
> Sent: Wednesday, April 16, 2025 3:38 AM
> 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: Dexuan Cui <decui@...rosoft.com>; KY Srinivasan <kys@...rosoft.com>;
> Paul Rosswurm <paulros@...rosoft.com>; olaf@...fle.de;
> vkuznets@...hat.com; davem@...emloft.net; wei.liu@...nel.org; Long Li
> <longli@...rosoft.com>; linux-kernel@...r.kernel.org
> Subject: [EXTERNAL] 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.

Thanks, I will use VM_BUG_ON() instead of a checking here. Also, will
keep the checking below unchanged.

> > +
> >  	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))