[Tarantool-patches] [PATCH 2/5] info: use luaL_pushuuidstr() for box.info uuids

Vladislav Shpilevoy v.shpilevoy at tarantool.org
Wed Jul 28 00:24:12 MSK 2021


box.info.uuid, box.info.cluster.uuid, and box.info.* replica UUIDs
used tt_uuid_str() function. The function is not safe to use in
preemptive context like Lua, where any attempt to push something
onto the Lua stack might trigger GC, which in turn might invoke
any other code.

It is not safe because uses the static buffer, which is global and
cyclic. Newer allocations can override the old data without any
warning.

Follow up #5632
Follow up #6050
Part of #6259
---
 src/box/lua/info.c                            |  8 ++--
 ...> gh-5632-6050-6259-gc-buf-reuse.test.lua} | 48 ++++++++++++++++---
 2 files changed, 46 insertions(+), 10 deletions(-)
 rename test/app-tap/{gh-5632-6050-gc-buf-reuse.test.lua => gh-5632-6050-6259-gc-buf-reuse.test.lua} (79%)

diff --git a/src/box/lua/info.c b/src/box/lua/info.c
index 1d8fe7938..d297ec6f6 100644
--- a/src/box/lua/info.c
+++ b/src/box/lua/info.c
@@ -176,7 +176,7 @@ lbox_pushreplica(lua_State *L, struct replica *replica)
 	lua_settable(L, -3);
 
 	lua_pushstring(L, "uuid");
-	lua_pushstring(L, tt_uuid_str(&replica->uuid));
+	luaL_pushuuidstr(L, &replica->uuid);
 	lua_settable(L, -3);
 
 	lua_pushstring(L, "lsn");
@@ -235,7 +235,7 @@ lbox_info_replication_anon_call(struct lua_State *L)
 		if (!replica->anon)
 			continue;
 
-		lua_pushstring(L, tt_uuid_str(&replica->uuid));
+		luaL_pushuuidstr(L, &replica->uuid);
 		lbox_pushreplica(L, replica);
 
 		lua_settable(L, -3);
@@ -290,7 +290,7 @@ lbox_info_id(struct lua_State *L)
 static int
 lbox_info_uuid(struct lua_State *L)
 {
-	lua_pushlstring(L, tt_uuid_str(&INSTANCE_UUID), UUID_STR_LEN);
+	luaL_pushuuidstr(L, &INSTANCE_UUID);
 	return 1;
 }
 
@@ -376,7 +376,7 @@ lbox_info_cluster(struct lua_State *L)
 {
 	lua_createtable(L, 0, 2);
 	lua_pushliteral(L, "uuid");
-	lua_pushlstring(L, tt_uuid_str(&REPLICASET_UUID), UUID_STR_LEN);
+	luaL_pushuuidstr(L, &REPLICASET_UUID);
 	lua_settable(L, -3);
 	return 1;
 }
diff --git a/test/app-tap/gh-5632-6050-gc-buf-reuse.test.lua b/test/app-tap/gh-5632-6050-6259-gc-buf-reuse.test.lua
similarity index 79%
rename from test/app-tap/gh-5632-6050-gc-buf-reuse.test.lua
rename to test/app-tap/gh-5632-6050-6259-gc-buf-reuse.test.lua
index bf7590a14..f806ba6b7 100755
--- a/test/app-tap/gh-5632-6050-gc-buf-reuse.test.lua
+++ b/test/app-tap/gh-5632-6050-6259-gc-buf-reuse.test.lua
@@ -1,10 +1,10 @@
 #!/usr/bin/env tarantool
 
 --
--- gh-5632, gh-6050: Lua code should not use any global buffers or objects
--- without proper ownership protection. Otherwise these items might be suddenly
--- reused during Lua GC which happens almost at any moment. That might lead to
--- data corruption.
+-- gh-5632, gh-6050, gh-6259: Lua code should not use any global buffers or
+-- objects without proper ownership protection. Otherwise these items might be
+-- suddenly reused during Lua GC which happens almost at any moment. That might
+-- lead to data corruption.
 --
 
 local tap = require('tap')
@@ -190,11 +190,47 @@ local function test_json(test)
    test:ok(is_success, 'json in gc')
 end
 
-local test = tap.test('gh-5632-6050-gc-buf-reuse')
-test:plan(4)
+local function test_info_uuid(test)
+    test:plan(1)
+
+    local gc_count = 100
+    local iter_count = 1000
+    local is_success = true
+
+    local function uuid_to_str()
+        local uu = uuid.new()
+        local str1 = box.info.uuid
+        local str2 = box.info.cluster.uuid
+        local str3 = box.info.uuid
+        local str4 = box.info.cluster.uuid
+        if str1 ~= str3 or str2 ~= str4 then
+            is_success = false
+            assert(false)
+        end
+    end
+
+    local function create_gc()
+        for _ = 1, gc_count do
+            ffi.gc(ffi.new('char[1]'), function() uuid_to_str() end)
+        end
+    end
+
+    for _ = 1, iter_count do
+        create_gc()
+        uuid_to_str()
+    end
+
+    test:ok(is_success, 'info uuid in gc')
+end
+
+box.cfg{}
+
+local test = tap.test('gh-5632-6050-6259-gc-buf-reuse')
+test:plan(5)
 test:test('uuid in __gc', test_uuid)
 test:test('uri in __gc', test_uri)
 test:test('msgpackffi in __gc', test_msgpackffi)
 test:test('json in __gc', test_json)
+test:test('info uuid in __gc', test_info_uuid)
 
 os.exit(test:check() and 0 or 1)
-- 
2.24.3 (Apple Git-128)



More information about the Tarantool-patches mailing list