Tarantool development patches archive
 help / color / mirror / Atom feed
From: Vladislav Shpilevoy via Tarantool-patches <tarantool-patches@dev.tarantool.org>
To: tarantool-patches@dev.tarantool.org, gorcunov@gmail.com,
	sergos@tarantool.org
Subject: [Tarantool-patches] [PATCH 2/5] info: use luaL_pushuuidstr() for box.info uuids
Date: Tue, 27 Jul 2021 23:24:12 +0200	[thread overview]
Message-ID: <e8087c2764119c8530a275091196dac4d12b7cb3.1627420835.git.v.shpilevoy@tarantool.org> (raw)
In-Reply-To: <cover.1627420835.git.v.shpilevoy@tarantool.org>

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)


  parent reply	other threads:[~2021-07-27 21:25 UTC|newest]

Thread overview: 20+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-27 21:24 [Tarantool-patches] [PATCH 0/5] Static buf in Lua, part 3 Vladislav Shpilevoy via Tarantool-patches
2021-07-27 21:24 ` [Tarantool-patches] [PATCH 1/5] uuid: introduce and use luaL_pushuuidstr() Vladislav Shpilevoy via Tarantool-patches
2021-07-29 11:30   ` Sergey Ostanevich via Tarantool-patches
2021-07-27 21:24 ` Vladislav Shpilevoy via Tarantool-patches [this message]
2021-07-29 11:38   ` [Tarantool-patches] [PATCH 2/5] info: use luaL_pushuuidstr() for box.info uuids Sergey Ostanevich via Tarantool-patches
2021-08-01 15:03     ` Vladislav Shpilevoy via Tarantool-patches
2021-08-01 17:01       ` Sergey Ostanevich via Tarantool-patches
2021-07-27 21:24 ` [Tarantool-patches] [PATCH 3/5] decimal: rename decimal_to_string to decimal_str Vladislav Shpilevoy via Tarantool-patches
2021-07-29 11:41   ` Sergey Ostanevich via Tarantool-patches
2021-08-01 15:03     ` Vladislav Shpilevoy via Tarantool-patches
2021-08-01 17:01       ` Sergey Ostanevich via Tarantool-patches
2021-07-27 21:24 ` [Tarantool-patches] [PATCH 4/5] decimal: introduce decimal_to_string Vladislav Shpilevoy via Tarantool-patches
2021-07-29 11:52   ` Sergey Ostanevich via Tarantool-patches
2021-08-01 15:04     ` Vladislav Shpilevoy via Tarantool-patches
2021-08-01 17:06       ` Sergey Ostanevich via Tarantool-patches
2021-07-27 21:24 ` [Tarantool-patches] [PATCH 5/5] decimal: introduce and use lua_pushdecimalstr() Vladislav Shpilevoy via Tarantool-patches
2021-07-29 12:28   ` Sergey Ostanevich via Tarantool-patches
2021-08-01 15:04     ` Vladislav Shpilevoy via Tarantool-patches
2021-07-27 21:39 ` [Tarantool-patches] [PATCH 0/5] Static buf in Lua, part 3 Cyrill Gorcunov via Tarantool-patches
2021-08-02 19:45 ` Vladislav Shpilevoy via Tarantool-patches

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=e8087c2764119c8530a275091196dac4d12b7cb3.1627420835.git.v.shpilevoy@tarantool.org \
    --to=tarantool-patches@dev.tarantool.org \
    --cc=gorcunov@gmail.com \
    --cc=sergos@tarantool.org \
    --cc=v.shpilevoy@tarantool.org \
    --subject='Re: [Tarantool-patches] [PATCH 2/5] info: use luaL_pushuuidstr() for box.info uuids' \
    /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