From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from smtp.kernel.org (aws-us-west-2-korg-mail-1.web.codeaurora.org [10.30.226.201]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.subspace.kernel.org (Postfix) with ESMTPS id 1B2FA373BE2 for ; Wed, 11 Mar 2026 16:59:07 +0000 (UTC) Authentication-Results: smtp.subspace.kernel.org; arc=none smtp.client-ip=10.30.226.201 ARC-Seal:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1773248348; cv=none; b=hm5IjtC3uqrcuYYMtQYedc7LLozfUNuLWUcrdrzIy79AT5vJIv8euLzN70TW94Z7xJWz7EBRpqWP1BphuaW92EUGWjZ3BBPH/4MmnLtD5sU6fbJjOT8U4QG7x8cybs/4kjI366k/saeVpQ9MJ30hPl4BJgdyPpciH9JIhYi+Pdk= ARC-Message-Signature:i=1; a=rsa-sha256; d=subspace.kernel.org; s=arc-20240116; t=1773248348; c=relaxed/simple; bh=LxU4Jmzb48ECfIuqmqY9+SrwRi4R0KnxC2jMteOCe2Y=; h=Message-ID:Date:MIME-Version:From:Subject:To:Cc:References: In-Reply-To:Content-Type; b=otJgnNDUXNIs2ngXqJwMy1oc24fdx+UVqhSAtiJ2Ii6HKdfTeg8KxVMIbTkvwAnrEyA3zOpQfAl4izQ4bJ0LWbjgfwdBYhN1+XvVnokC/Gfr2VDLxM1PYRoT5mLScPYsK+8waqt9wrOmAHBICIuTbfjdqOPuBW8dKkYHNED7YjM= ARC-Authentication-Results:i=1; smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b=lMmXpJ7u; arc=none smtp.client-ip=10.30.226.201 Authentication-Results: smtp.subspace.kernel.org; dkim=pass (2048-bit key) header.d=kernel.org header.i=@kernel.org header.b="lMmXpJ7u" Received: by smtp.kernel.org (Postfix) with ESMTPSA id AF58AC4CEF7; Wed, 11 Mar 2026 16:59:05 +0000 (UTC) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=kernel.org; s=k20201202; t=1773248347; bh=LxU4Jmzb48ECfIuqmqY9+SrwRi4R0KnxC2jMteOCe2Y=; h=Date:From:Subject:To:Cc:References:In-Reply-To:From; b=lMmXpJ7uxf0MVzwxwAT6gSFDWQu4bT/fhhtR0TSooudyOHbNkFA0faH9lwiAX/Ybv hsrE0g/ShaDhSJGCmu5htgQoXY13cFtDQmUZkhsAYqJdnoZIyqGqfZvTPK0ChAMRYc Q2dpbDdTB14iUOvhByk6iA9pLdUedS9bFxGpFyQQGErMGXW/cXEoGaCmyIEohs72aK fRxJ/HLbPrZ3iJ4YoNtZD+dszZ0bZ6bqOTJ5MoLs+piZhDQx9mrBdPbkgFC2DEbns4 SMpxLyorOE0PFnw0fW/kYN44S+QWhRwIZthigs64+FNn9mig0ue9PiqsSPtYRCYVqs US4TGh46xZWgA== Message-ID: <7791ab6a-4707-44b4-a868-d88b93502b1f@kernel.org> Date: Wed, 11 Mar 2026 17:59:03 +0100 Precedence: bulk X-Mailing-List: linux-kernel@vger.kernel.org List-Id: List-Subscribe: List-Unsubscribe: MIME-Version: 1.0 User-Agent: Mozilla Thunderbird From: Vlastimil Babka Subject: Re: [PATCH] slab: fix memory leak when refill_sheaf() fails Content-Language: en-US To: Harry Yoo , Qing Wang Cc: Andrew Morton , Hao Li , Christoph Lameter , David Rientjes , Roman Gushchin , Suren Baghdasaryan , linux-mm@kvack.org, linux-kernel@vger.kernel.org References: <20260311093617.4155965-1-wangqing7171@gmail.com> In-Reply-To: Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 7bit On 3/11/26 12:16, Harry Yoo wrote: > 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. > > Nice catch, thanks! Indeed, thanks! > Probably the need to fail new_slab() made it quite hard to trigger and notice. Agreed. >> 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") Actually I think that commit was fine as it was using bulk alloc to refill and that was undoing any partial successes. I think this one is correct and replaced it so: Fixes: ed30c4adfc2b ("slab: add optimized sheaf refill from partial list") > I think we need to add Cc: stable@vger.kernel.org And therefore we don't, unless I'm mistaken. >> Signed-off-by: Qing Wang >> --- >> mm/slub.c | 4 ++++ >> 1 file changed, 4 insertions(+) >> >> diff --git a/mm/slub.c b/mm/slub.c >> index 20cb4f3b636d..73b2cfd0e123 100644 >> --- a/mm/slub.c >> +++ b/mm/slub.c >> @@ -2797,6 +2797,7 @@ static void free_empty_sheaf(struct kmem_cache *s, struct slab_sheaf *sheaf) >> if (s->flags & SLAB_KMALLOC) >> mark_obj_codetag_empty(sheaf); >> >> + WARN_ON(sheaf->size > 0); > > nit: perhaps VM_WARN_ON_ONCE(); will be enough? Yep replaced it too. Added to slab/for-next-fixes, thanks! > Otherwise looks good to me, so: > Reviewed-by: Harry Yoo >