BPF List
 help / color / mirror / Atom feed
From: Christoph Hellwig <hch@lst.de>
To: sashiko-reviews@lists.linux.dev
Cc: Christoph Hellwig <hch@lst.de>, bpf@vger.kernel.org
Subject: Re: [PATCH] mm/slab: improve kmem_cache_alloc_bulk
Date: Wed, 27 May 2026 10:06:24 +0200	[thread overview]
Message-ID: <20260527080624.GA20439@lst.de> (raw)
In-Reply-To: <20260527072718.42A141F000E9@smtp.kernel.org>

On Wed, May 27, 2026 at 07:27:17AM +0000, sashiko-bot@kernel.org wrote:
> Does this omit setting op_ctx->rsvd_page_tables.count to pt_count on success?
> Since op_ctx is initialized via memset previously, the count will remain 0,
> which could lead to memory leaks when these page tables are consumed or
> cleaned up later.

Yes, fixed.

> >  			} else {
> >  				int i;
> > -				for (i = 0; i < ret; i++)
> > +				for (i = 0; i < BULK_SIZE; i++)
> >  					fail |= check_buf(bulk_array[i], size, want_ctor, want_rcu, want_zero);
> >  				kmem_cache_free_bulk(c, ret, bulk_array);
> 
> [Severity: Low]
> Will this cause a compilation error? The local variable ret is removed from
> this scope, but it is still used in the kmem_cache_free_bulk() call above.

Yes, no idea why this didn't get included in my build after
enabling it earlier.

> 
> >  			}
> > @@ -354,17 +352,18 @@ static int __init do_kmem_cache_size_bulk(int size, int *total_failures)
> >  
> >  	c = kmem_cache_create("test_cache", size, size, 0, NULL);
> >  	for (iter = 0; (iter < maxiter) && !fail; iter++) {
> > -		num = kmem_cache_alloc_bulk(c, GFP_KERNEL, ARRAY_SIZE(objects),
> > -					    objects);
> > -		for (i = 0; i < num; i++) {
> > +		if (!kmem_cache_alloc_bulk(c, GFP_KERNEL, ARRAY_SIZE(objects),
> > +				objects))
> > +			continue;
> > +
> > +		for (i = 0; i < ARRAY_SIZE(objects); i++) {
> >  			bytes = count_nonzero_bytes(objects[i], size);
> >  			if (bytes)
> >  				fail = true;
> >  			fill_with_garbage(objects[i], size);
> >  		}
> >  
> > -		if (num)
> > -			kmem_cache_free_bulk(c, num, objects);
> > +		kmem_cache_free_bulk(c, num, objects);
> 
> [Severity: Medium]
> Is num uninitialized here? It's no longer assigned the result of the
> allocation call, so passing it to kmem_cache_free_bulk() might use
> indeterminate stack data and lead to memory corruption.

Yes, this should use ARRAY_SIZE directly as well.

> > +bool kmem_cache_alloc_bulk_noprof(struct kmem_cache *s, gfp_t flags,
> > +		size_t size, void **p)
> >  {
> >  	unsigned int i = 0;
> >  	void *kfence_obj;
> >  
> >  	if (!size)
> > -		return 0;
> > +		return false;
> 
> [Severity: High]
> Does this incorrectly report an allocation failure when size is 0?

It correctly reports one as that's how all callers treat a 0 return.

Update version in git here, not reposting until more human comments
arrive:

https://git.infradead.org/?p=users/hch/misc.git;a=shortlog;h=refs/heads/fix-kmem_cache_alloc_bulk

  reply	other threads:[~2026-05-27  8:06 UTC|newest]

Thread overview: 33+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-27  7:02 improve the kmem_cache_alloc_bulk API Christoph Hellwig
2026-05-27  7:02 ` [PATCH] mm/slab: improve kmem_cache_alloc_bulk Christoph Hellwig
2026-05-27  7:27   ` sashiko-bot
2026-05-27  8:06     ` Christoph Hellwig [this message]
2026-05-27  7:53   ` bot+bpf-ci
2026-05-27  8:51   ` Jesper Dangaard Brouer
2026-05-27 13:56     ` Alexander Lobakin
2026-05-27 14:07       ` Christoph Hellwig
2026-05-27  9:38   ` Vlastimil Babka (SUSE)
2026-05-27 12:20     ` Christoph Hellwig
2026-05-28  8:58   ` kernel test robot
2026-05-27  9:11 ` improve the kmem_cache_alloc_bulk API Vlastimil Babka (SUSE)
2026-05-27 12:21   ` Christoph Hellwig
2026-05-27 14:07     ` Vlastimil Babka (SUSE)
2026-05-28  9:05       ` Christoph Hellwig
2026-05-28  9:16         ` Vlastimil Babka (SUSE)
  -- strict thread matches above, loose matches on Subject: below --
2026-05-28  9:34 improve the kmem_cache_alloc_bulk API v2 Christoph Hellwig
2026-05-28  9:34 ` [PATCH] mm/slab: improve kmem_cache_alloc_bulk Christoph Hellwig
2026-05-28 10:12   ` sashiko-bot
2026-05-29 11:54   ` Vlastimil Babka (SUSE)
2026-05-29 13:50     ` Christoph Hellwig
2026-06-01  6:39       ` Harry Yoo
2026-06-01  8:16         ` Vlastimil Babka (SUSE)
2026-06-01 11:38           ` Christoph Hellwig
2026-06-01 12:50             ` Vlastimil Babka (SUSE)
2026-06-01 13:32               ` Rob Clark
2026-06-01 14:39                 ` Rob Clark
2026-06-03  9:17                   ` Vlastimil Babka (SUSE)
2026-06-03 11:13                     ` Rob Clark
2026-06-03 16:22                       ` Vlastimil Babka (SUSE)
2026-06-04  7:10                         ` Harry Yoo
2026-06-04  7:35                           ` Vlastimil Babka (SUSE)
2026-06-04  9:36                             ` Rob Clark
2026-06-01 11:39           ` Harry Yoo

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=20260527080624.GA20439@lst.de \
    --to=hch@lst.de \
    --cc=bpf@vger.kernel.org \
    --cc=sashiko-reviews@lists.linux.dev \
    /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