Tarantool development patches archive
 help / color / mirror / Atom feed
From: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
To: tarantool-patches@dev.tarantool.org, korablev@tarantool.org
Subject: [Tarantool-patches] [PATCH small 1/1] lsregion: fix slab_unmap() called on malloced slab
Date: Fri, 15 May 2020 00:31:53 +0200	[thread overview]
Message-ID: <c12798db852383743672787e38aa0e2f88844172.1589495493.git.v.shpilevoy@tarantool.org> (raw)

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)

             reply	other threads:[~2020-05-14 22:31 UTC|newest]

Thread overview: 4+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-05-14 22:31 Vladislav Shpilevoy [this message]
2020-05-19 16:21 ` Aleksandr Lyapunov
2020-05-23 18:13   ` Konstantin Osipov
2020-05-24 14:06     ` Vladislav Shpilevoy

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=c12798db852383743672787e38aa0e2f88844172.1589495493.git.v.shpilevoy@tarantool.org \
    --to=v.shpilevoy@tarantool.org \
    --cc=korablev@tarantool.org \
    --cc=tarantool-patches@dev.tarantool.org \
    --subject='Re: [Tarantool-patches] [PATCH small 1/1] lsregion: fix slab_unmap() called on malloced slab' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox