From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from smtp29.i.mail.ru (smtp29.i.mail.ru [94.100.177.89]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id 085D9469710 for ; Fri, 15 May 2020 01:31:55 +0300 (MSK) From: Vladislav Shpilevoy Date: Fri, 15 May 2020 00:31:53 +0200 Message-Id: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Subject: [Tarantool-patches] [PATCH small 1/1] lsregion: fix slab_unmap() called on malloced slab List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , To: tarantool-patches@dev.tarantool.org, korablev@tarantool.org Lsregion allocates slabs using either - Slab_map() from slab arena, when allocation size is smaller, than slab size; - Using cached slab, stored in the lsregion as a protection from oscillation; - Using malloc(), when requested size is too big. Malloc() was used when allocation size was >= fixed slab size - meta size. However free() was used, when real slab size was > fixed slab size - meta size. So if an allocation was exactly of size 'fixed slab size - meta size', it was allocated using malloc(), but freed using slab_unmap(). That lead to a crash, if 'lucky'. But as it is a memory corruption, could lead to anything. --- Branch: http://github.com/tarantool/small/tree/gerold103/fix-lsregion-crash-or-leak This led to at least leaks in vinyl. Since it used lsregion very extensively for 0 level of LSM trees. small/lsregion.c | 2 +- test/lsregion.c | 7 +++++++ 2 files changed, 8 insertions(+), 1 deletion(-) diff --git a/small/lsregion.c b/small/lsregion.c index b7f2684..6bcc043 100644 --- a/small/lsregion.c +++ b/small/lsregion.c @@ -45,7 +45,7 @@ lsregion_alloc_slow(struct lsregion *lsregion, size_t size, int64_t id) } if ((slab != NULL && size > lslab_unused(slab)) || slab == NULL) { - if (size + lslab_sizeof() >= slab_size) { + if (size + lslab_sizeof() > slab_size) { /* Large allocation, use malloc() */ slab_size = size + lslab_sizeof(); struct quota *quota = lsregion->arena->quota; diff --git a/test/lsregion.c b/test/lsregion.c index 1ed6a3a..90ad060 100644 --- a/test/lsregion.c +++ b/test/lsregion.c @@ -140,6 +140,13 @@ test_basic() is(lsregion_slab_count(&allocator), 0, "slab count after large gc()"); + /* + * Allocate exactly slab size. + */ + ++id; + data = lsregion_alloc(&allocator, arena.slab_size - lslab_sizeof(), id); + lsregion_gc(&allocator, id); + lsregion_destroy(&allocator); /* Sic: slabs are cached by arena */ is(arena.used, 2 * arena.slab_size, "arena used after destroy"); -- 2.21.1 (Apple Git-122.3)