From: Alexander Turenko <alexander.turenko@tarantool.org> To: Vladislav Shpilevoy <v.shpilevoy@tarantool.org> Cc: tarantool-patches@dev.tarantool.org, Alexander Turenko <alexander.turenko@tarantool.org> Subject: [Tarantool-patches] [PATCH] lua/merger: fix NULL dereferences on error paths Date: Tue, 10 Nov 2020 11:13:14 +0300 [thread overview] Message-ID: <ed3a462eab028de11cea256c8f0efe5bb1ff2c0c.1604995527.git.alexander.turenko@tarantool.org> (raw) There are two situations that may lead to a NULL dereference. First, when an attempt to create a merge source is failed. Now only OOM may trigger this code path. Reproducing it in a test is tricky, so I left it without a test case. Second, when an attempt to create a merger is failed. The reason of the fail may be a 'wrong' key_def: when we unable to create a tuple format from it. Say, when the key_def defines several key parts using the same field, but marks them with different field types. Fixes #5450 --- https://github.com/tarantool/tarantool/issues/5450 Totktonada/gh-5450-merger-segfault-on-wrong-key-def https://github.com/tarantool/tarantool/tree/Totktonada/gh-5450-merger-segfault-on-wrong-key-def CI is not sufficient now, because of [1]. Aside engine/errinj_ddl.test.lua, all tests are passed on my Linux machine. [1]: https://github.com/tarantool/tarantool/issues/5473 src/box/lua/merger.c | 8 ++---- .../gh-5450-merger-wrong-key-def.test.lua | 26 +++++++++++++++++++ 2 files changed, 28 insertions(+), 6 deletions(-) create mode 100755 test/box-tap/gh-5450-merger-wrong-key-def.test.lua diff --git a/src/box/lua/merger.c b/src/box/lua/merger.c index 17e237902..89bdfffa6 100644 --- a/src/box/lua/merger.c +++ b/src/box/lua/merger.c @@ -266,10 +266,8 @@ lbox_merge_source_new(struct lua_State *L, const char *func_name, lua_pushnil(L); struct merge_source *source = luaL_merge_source_new(L); - if (source == NULL) { - merge_source_unref(source); + if (source == NULL) return luaT_error(L); - } *(struct merge_source **) luaL_pushcdata(L, CTID_STRUCT_MERGE_SOURCE_REF) = source; lua_pushcfunction(L, lbox_merge_source_gc); @@ -387,10 +385,8 @@ lbox_merger_new(struct lua_State *L) struct merge_source *merger = merger_new(key_def, sources, source_count, reverse); free(sources); - if (merger == NULL) { - merge_source_unref(merger); + if (merger == NULL) return luaT_error(L); - } *(struct merge_source **) luaL_pushcdata(L, CTID_STRUCT_MERGE_SOURCE_REF) = merger; diff --git a/test/box-tap/gh-5450-merger-wrong-key-def.test.lua b/test/box-tap/gh-5450-merger-wrong-key-def.test.lua new file mode 100755 index 000000000..033f42da4 --- /dev/null +++ b/test/box-tap/gh-5450-merger-wrong-key-def.test.lua @@ -0,0 +1,26 @@ +#!/usr/bin/env tarantool + +local tap = require('tap') +local key_def_lib = require('key_def') +local merger = require('merger') + +local test = tap.test('gh-5450-merger-wrong-key-def') +test:plan(1) + +-- gh-5450: creating of merger leads to segfault, when passed +-- key_def is not usable to create a tuple format. +local key_def = key_def_lib.new({ + {fieldno = 6, type = 'string'}, + {fieldno = 6, type = 'unsigned'}, +}) +local exp_err = "Field 6 has type 'string' in one index, but type " .. + "'unsigned' in another" +-- At the moment of writting the test, it is sufficient to pass an +-- empty sources table to trigger the attempt to create a tuple +-- format. +local sources = {} +local ok, err = pcall(merger.new, key_def, sources) +test:is_deeply({ok, tostring(err)}, {false, exp_err}, + 'unable to create a tuple format') + +os.exit(test:check() and 0 or 1) -- 2.25.0
next reply other threads:[~2020-11-10 8:13 UTC|newest] Thread overview: 8+ messages / expand[flat|nested] mbox.gz Atom feed top 2020-11-10 8:13 Alexander Turenko [this message] 2020-11-16 21:22 ` Vladislav Shpilevoy 2020-11-16 22:19 ` Alexander Turenko 2020-11-17 8:35 ` Timur Safin 2020-11-17 8:38 ` Timur Safin 2020-11-17 12:25 ` Alexander Turenko 2020-11-18 12:27 ` Alexander V. Tikhonov 2020-11-19 8:15 ` Alexander Turenko
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=ed3a462eab028de11cea256c8f0efe5bb1ff2c0c.1604995527.git.alexander.turenko@tarantool.org \ --to=alexander.turenko@tarantool.org \ --cc=tarantool-patches@dev.tarantool.org \ --cc=v.shpilevoy@tarantool.org \ --subject='Re: [Tarantool-patches] [PATCH] lua/merger: fix NULL dereferences on error paths' \ /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