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)
next 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