linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Harry Yoo <harry.yoo@oracle.com>
To: I Viswanath <viswanathiyyappan@gmail.com>
Cc: vbabka@suse.cz, akpm@linux-foundation.org, cl@gentwo.org,
	rientjes@google.com, roman.gushchin@linux.dev,
	linux-mm@kvack.org, linux-kernel@vger.kernel.org,
	skhan@linuxfoundation.org, david.hunter.linux@gmail.com,
	linux-kernel-mentees@lists.linux.dev,
	syzbot+94d20db923b9f51be0df@syzkaller.appspotmail.com
Subject: Re: [RFC PATCH] mm/slab: Add size validation in kmalloc_array_* functions
Date: Tue, 23 Sep 2025 02:59:56 +0900	[thread overview]
Message-ID: <aNGOnDb0WsIbk2zx@hyeyoo> (raw)
In-Reply-To: <20250922170357.148588-1-viswanathiyyappan@gmail.com>

Hi I, thanks for looking into the syzbot report.

On Mon, Sep 22, 2025 at 10:33:57PM +0530, I Viswanath wrote:
> syzbot reported WARNING in max_vclocks_store.
> 
> This occurs when the size argument fits into a u32 but is too large 
> to allocate, i.e., when it's between KMALLOC_MAX_SIZE + 1 
> and UINT_MAX (both limits included)

This is not quite.

When bytes > KKMALLOC_MAX_SIZE (8K on my system), kmalloc redirects allocation
to the buddy allocator, which can allocate up to (PAGE_SIZE << MAX_PAGE_ORDER)
bytes (4M on my system).

Because allocating a page with order > MAX_PAGE_ORDER page is never
supposed to succeed, the caller of kmalloc should be fixed rather than
kmalloc itself.

> Add validation to kmalloc_array_noprof() and related functions to 
> return early if the requested size exceeds KMALLOC_MAX_SIZE.

This is against the point of WARNING in the buddy allocator.

I think the right fix should be to return -EINVAL in max_vclocks_store()
if max * sizeof(int) exceeds PAGE_SIZE << MAX_PAGE_ORDER?

-- 
Cheers,
Harry / Hyeonggon

> This seems like the most reasonable place for this guard.
> 
> Would it be a good idea to move the check down to 
> the lower level functions like __kmalloc_node_noprof()?
> 
> Moving it up is not a good idea because
> max_vclocks_store shouldn't reason around KMALLOC_MAX_SIZE, 
> a mm specific macro.
> 
> Should the Fixes: commit here be the one in which this file 
> was added?
> 
> Reported-by: syzbot+94d20db923b9f51be0df@syzkaller.appspotmail.com
> Tested-by: syzbot+94d20db923b9f51be0df@syzkaller.appspotmail.com
> Closes: https://syzkaller.appspot.com/bug?extid=94d20db923b9f51be0df
> Fixes: 1da177e4c3f4 ("Linux-2.6.12-rc2")
> Signed-off-by: I Viswanath <viswanathiyyappan@gmail.com>
> ---
>  include/linux/slab.h | 8 ++++----
>  1 file changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/include/linux/slab.h b/include/linux/slab.h
> index d5a8ab98035c..6db15c5b2ce7 100644
> --- a/include/linux/slab.h
> +++ b/include/linux/slab.h
> @@ -943,7 +943,7 @@ static inline __alloc_size(1, 2) void *kmalloc_array_noprof(size_t n, size_t siz
>  {
>  	size_t bytes;
>  
> -	if (unlikely(check_mul_overflow(n, size, &bytes)))
> +	if (unlikely(check_mul_overflow(n, size, &bytes) || (bytes > KMALLOC_MAX_SIZE)))
>  		return NULL;
>  	return kmalloc_noprof(bytes, flags);
>  }
> @@ -973,7 +973,7 @@ static inline __realloc_size(2, 3) void * __must_check krealloc_array_noprof(voi
>  {
>  	size_t bytes;
>  
> -	if (unlikely(check_mul_overflow(new_n, new_size, &bytes)))
> +	if (unlikely(check_mul_overflow(new_n, new_size, &bytes) || (bytes > KMALLOC_MAX_SIZE)))
>  		return NULL;
>  
>  	return krealloc_noprof(p, bytes, flags);
> @@ -1013,7 +1013,7 @@ static inline __alloc_size(1, 2) void *kmalloc_array_node_noprof(size_t n, size_
>  {
>  	size_t bytes;
>  
> -	if (unlikely(check_mul_overflow(n, size, &bytes)))
> +	if (unlikely(check_mul_overflow(n, size, &bytes) || (bytes > KMALLOC_MAX_SIZE)))
>  		return NULL;
>  	if (__builtin_constant_p(n) && __builtin_constant_p(size))
>  		return kmalloc_node_noprof(bytes, flags, node);
> @@ -1059,7 +1059,7 @@ kvmalloc_array_node_noprof(size_t n, size_t size, gfp_t flags, int node)
>  {
>  	size_t bytes;
>  
> -	if (unlikely(check_mul_overflow(n, size, &bytes)))
> +	if (unlikely(check_mul_overflow(n, size, &bytes) || (bytes > KMALLOC_MAX_SIZE)))
>  		return NULL;
>  
>  	return kvmalloc_node_noprof(bytes, flags, node);
> -- 
> 2.47.3
> 


  reply	other threads:[~2025-09-22 18:00 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-09-22 17:03 I Viswanath
2025-09-22 17:59 ` Harry Yoo [this message]
2025-09-23  5:11   ` viswanath
2025-09-23  6:14     ` Harry Yoo
2025-09-22 21:36 ` Matthew Wilcox
2025-09-23  4:28   ` viswanath

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aNGOnDb0WsIbk2zx@hyeyoo \
    --to=harry.yoo@oracle.com \
    --cc=akpm@linux-foundation.org \
    --cc=cl@gentwo.org \
    --cc=david.hunter.linux@gmail.com \
    --cc=linux-kernel-mentees@lists.linux.dev \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=rientjes@google.com \
    --cc=roman.gushchin@linux.dev \
    --cc=skhan@linuxfoundation.org \
    --cc=syzbot+94d20db923b9f51be0df@syzkaller.appspotmail.com \
    --cc=vbabka@suse.cz \
    --cc=viswanathiyyappan@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox