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