[Tarantool-patches] [PATCH] lua/merger: fix NULL dereferences on error paths

[Alexander Turenko]

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