Re: [External] Re: [PATCH v5.4.y, v4.19.y] mm: memcg/slab: fix memory leak at non-root kmem_cache destroy

From: Vlastimil Babka <vbabka@suse.cz>
To: Muchun Song <songmuchun@bytedance.com>
Cc: Christoph Lameter <cl@linux.com>,
	Pekka Enberg <penberg@kernel.org>,
	David Rientjes <rientjes@google.com>,
	Joonsoo Kim <iamjoonsoo.kim@lge.com>,
	Andrew Morton <akpm@linux-foundation.org>,
	Linux Memory Management List <linux-mm@kvack.org>,
	LKML <linux-kernel@vger.kernel.org>,
	Shakeel Butt <shakeelb@google.com>, Roman Gushchin <guro@fb.com>
Subject: Re: [External] Re: [PATCH v5.4.y, v4.19.y] mm: memcg/slab: fix memory leak at non-root kmem_cache destroy
Date: Wed, 15 Jul 2020 17:43:08 +0200	[thread overview]
Message-ID: <4c1bba23-56c5-f69c-28cb-48dd3db30880@suse.cz> (raw)
In-Reply-To: <CAMZfGtXK9yQOJy7BPnTBzhG4tithRs=9R4O3rDg1Rjz0zUFKnQ@mail.gmail.com>

On 7/15/20 5:13 PM, Muchun Song wrote:
> On Wed, Jul 15, 2020 at 7:32 PM Vlastimil Babka <vbabka@suse.cz> wrote:
>>
>> On 7/7/20 8:27 AM, Muchun Song wrote:
>> > If the kmem_cache refcount is greater than one, we should not
>> > mark the root kmem_cache as dying. If we mark the root kmem_cache
>> > dying incorrectly, the non-root kmem_cache can never be destroyed.
>> > It resulted in memory leak when memcg was destroyed. We can use the
>> > following steps to reproduce.
>> >
>> >   1) Use kmem_cache_create() to create a new kmem_cache named A.
>> >   2) Coincidentally, the kmem_cache A is an alias for kmem_cache B,
>> >      so the refcount of B is just increased.
>> >   3) Use kmem_cache_destroy() to destroy the kmem_cache A, just
>> >      decrease the B's refcount but mark the B as dying.
>> >   4) Create a new memory cgroup and alloc memory from the kmem_cache
>> >      A. It leads to create a non-root kmem_cache for allocating.
>> >   5) When destroy the memory cgroup created in the step 4), the
>> >      non-root kmem_cache can never be destroyed.
>> >
>> > If we repeat steps 4) and 5), this will cause a lot of memory leak.
>> > So only when refcount reach zero, we mark the root kmem_cache as dying.
>> >
>> > Fixes: 92ee383f6daa ("mm: fix race between kmem_cache destroy, create and deactivate")
>> > Signed-off-by: Muchun Song <songmuchun@bytedance.com>
>>
>> CC Roman, who worked in this area recently.
>>
>> Also why is this marked "[PATCH v5.4.y, v4.19.y]"? Has it been fixed otherwise
>> in 5.5+ ?
> 
> Because the memcg slab/slub is reworked by Roman since v5.8.

That rework is in mmotm, so scheduled for 5.9, AFAIK. If you mean "The new
cgroup slab memory controller" series.

> Therefore, this problem exists in v5.7 and below.

Even 5.7 has a stable series, so no need to list only the LTS's.
To sum up, the patch (once reviewed) should be queued for mainline as usual,
perhaps sent before 5.8 is final, if deemed safe enough, and with added

Cc: <stable@vger.kernel.org>

and the Fixes: tag you provided, the applicable stable versions will pick it.

Vlastimil