linux-mm.kvack.org archive mirror
 help / color / mirror / Atom feed
From: Hao Li <hao.li@linux.dev>
To: Qing Wang <wangqing7171@gmail.com>,
	 Vlastimil Babka <vbabka@kernel.org>,
	Harry Yoo <harry.yoo@oracle.com>
Cc: Andrew Morton <akpm@linux-foundation.org>,
	 Christoph Lameter <cl@gentwo.org>,
	David Rientjes <rientjes@google.com>,
	 Roman Gushchin <roman.gushchin@linux.dev>,
	Suren Baghdasaryan <surenb@google.com>,
	linux-mm@kvack.org,  linux-kernel@vger.kernel.org
Subject: Re: [PATCH] slab: fix memory leak when refill_sheaf() fails
Date: Wed, 11 Mar 2026 22:45:59 +0800	[thread overview]
Message-ID: <thhmlnu6phtuucz7jo3xoqeiv64rs4ynkt63qoxvtwgfh3aofw@lo75dus27vyk> (raw)
In-Reply-To: <20260311093617.4155965-1-wangqing7171@gmail.com>

On Wed, Mar 11, 2026 at 05:36:17PM +0800, Qing Wang wrote:
> When refill_sheaf() partially fills one sheaf (e.g., fills 5 objects
> but need to fill 10), it will update sheaf->size and return -ENOMEM.
> However, the callers (alloc_full_sheaf() and __pcs_replace_empty_main())
> directly call free_empty_sheaf() on failure, which only does kfree(sheaf),
> causing the partially allocated objects memory in sheaf->objects[] leaked.
> 
> Fix this by calling sheaf_flush_unused() before free_empty_sheaf() to
> free objects of sheaf->objects[]. And also add a WARN_ON() in
> free_empty_sheaf() to catch any future cases where a non-empty sheaf is
> being freed.
> 
> Fixes: 2d517aa09bbc ("slab: add opt-in caching layer of percpu sheaves")
> Signed-off-by: Qing Wang <wangqing7171@gmail.com>
> ---

This fix looks good to me.
So feel free to add:

Reviewed-by: Hao Li <hao.li@linux.dev>

I also want to bring up another point here, although it may be outside the
scope of the current fix.

When I looked into the refill_sheaf() path, I found a refill failure does not
guarantee that the sheaf remains intact: refill_sheaf() can partially fill the
sheaf before failing. This non-intact behavior propagates to its caller,
__prefill_sheaf_pfmemalloc(), which therefore also cannot assume that the sheaf
is still intact after a refill failure.

However, the comment for kmem_cache_refill_sheaf() says that "if the refill
fails (returning -ENOMEM), the existing sheaf is left intact." That means the
behavior of __prefill_sheaf_pfmemalloc() - where the sheaf may be left
partially filled on refill failure - contradicts the API contract of
kmem_cache_refill_sheaf().

Maybe we can add rollback logic to __prefill_sheaf_pfmemalloc() so that it
provides intact semantics, preventing the non-intact behavior of refill_sheaf()
from propagating up to kmem_cache_refill_sheaf().

-- 
Thanks,
Hao


  parent reply	other threads:[~2026-03-11 14:46 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-03-11  9:36 Qing Wang
2026-03-11 11:16 ` Harry Yoo
2026-03-11 11:48   ` Harry Yoo
2026-03-12  2:21     ` Qing Wang
2026-03-12  3:35       ` Harry Yoo
2026-03-11 16:59   ` Vlastimil Babka
2026-03-12  3:28     ` Harry Yoo
2026-03-11 14:45 ` Hao Li [this message]
2026-03-11 16:30   ` Hao Li
2026-03-11 16:54     ` Vlastimil Babka
2026-03-12  4:40       ` Harry Yoo
2026-03-12  4:56         ` Hao Li

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=thhmlnu6phtuucz7jo3xoqeiv64rs4ynkt63qoxvtwgfh3aofw@lo75dus27vyk \
    --to=hao.li@linux.dev \
    --cc=akpm@linux-foundation.org \
    --cc=cl@gentwo.org \
    --cc=harry.yoo@oracle.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-mm@kvack.org \
    --cc=rientjes@google.com \
    --cc=roman.gushchin@linux.dev \
    --cc=surenb@google.com \
    --cc=vbabka@kernel.org \
    --cc=wangqing7171@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