From 5576ee3b6c73bb828c4492cd9e0e21ee3778dc95 Mon Sep 17 00:00:00 2001
From: Nikolay Shirokovskiy <nshirokovskiy@tarantool.org>
Date: Tue, 4 Jul 2023 12:56:15 +0300
Subject: [PATCH] asan: prepare for ASAN-friendly ibuf
ASAN-friendly implementation poisons memory after allocation with
ibuf_alloc so we need to fix existing places in code where we access
memory after allocation.
Part of ibuf implementation is inline functions in headers. Thus ibuf
implementation in Lua reimplement this parts. We add poison to these
inline functions in ASAN-friedly implementation so we need add same poison
in Lua implementation.
Part of #7327
NO_CHANGELOG=internal
NO_DOC=internal
(cherry picked from commit 4f542bb7f79a33f114d2bdfdf3e84ab8c3299f48)
---
src/box/iproto.cc | 3 +-
src/lua/buffer.lua | 18 +++++++
src/lua/init.c | 2 +-
src/lua/utils.lua | 33 +++++++++++++
test/app-luatest/buffer_test.lua | 81 ++++++++++++++++++++++++++++++++
test/box-tap/merger.test.lua | 4 +-
6 files changed, 137 insertions(+), 4 deletions(-)
create mode 100644 test/app-luatest/buffer_test.lua
diff --git a/src/box/iproto.cc b/src/box/iproto.cc
index a1419233b9..f2952812f0 100644
--- a/src/box/iproto.cc
+++ b/src/box/iproto.cc
@@ -1322,6 +1322,7 @@ iproto_connection_on_input(ev_loop *loop, struct ev_io *watcher,
return;
}
/* Read input. */
+ ibuf_reserve(in, ibuf_unused(in));
ssize_t nrd = iostream_read(io, in->wpos, ibuf_unused(in));
if (nrd < 0) { /* Socket is not ready. */
if (nrd == IOSTREAM_ERROR)
@@ -1343,7 +1344,7 @@ iproto_connection_on_input(ev_loop *loop, struct ev_io *watcher,
IPROTO_RECEIVED, nrd);
/* Update the read position and connection state. */
- in->wpos += nrd;
+ ibuf_alloc(in, nrd);
con->parse_size += nrd;
/* Enqueue all requests which are fully read up. */
if (iproto_enqueue_batch(con, in) != 0)
diff --git a/src/lua/buffer.lua b/src/lua/buffer.lua
index c5cefe32b6..9fc5ee8fda 100644
--- a/src/lua/buffer.lua
+++ b/src/lua/buffer.lua
@@ -1,6 +1,7 @@
-- buffer.lua (internal file)
local ffi = require('ffi')
+local utils = require('internal.utils')
local READAHEAD = 16320
ffi.cdef[[
@@ -120,10 +121,19 @@ local function ibuf_recycle(buf)
builtin.ibuf_reinit(buf)
end
+local function ibuf_poison_unallocated(buf)
+ utils.poison_memory_region(buf.wpos, buf.epos - buf.wpos);
+end
+
+local function ibuf_unpoison_unallocated(buf)
+ utils.unpoison_memory_region(buf.wpos, buf.epos - buf.wpos);
+end
+
local function ibuf_reset(buf)
checkibuf(buf, 'reset')
buf.rpos = buf.buf
buf.wpos = buf.buf
+ ibuf_poison_unallocated(buf)
end
local function ibuf_reserve_slow(buf, size)
@@ -137,6 +147,7 @@ end
local function ibuf_reserve(buf, size)
checkibuf(buf, 'reserve')
if buf.wpos + size <= buf.epos then
+ ibuf_unpoison_unallocated(buf)
return buf.wpos
end
return ibuf_reserve_slow(buf, size)
@@ -146,11 +157,18 @@ local function ibuf_alloc(buf, size)
checkibuf(buf, 'alloc')
local wpos
if buf.wpos + size <= buf.epos then
+ --
+ -- In case of using same buffer we need to unpoison newly
+ -- allocated memory after previous ibuf_alloc or poison after
+ -- newly allocated memory after previous ibuf_reserve.
+ --
+ ibuf_unpoison_unallocated(buf)
wpos = buf.wpos
else
wpos = ibuf_reserve_slow(buf, size)
end
buf.wpos = buf.wpos + size
+ ibuf_poison_unallocated(buf)
return wpos
end
diff --git a/src/lua/init.c b/src/lua/init.c
index 354fa52e0d..10cbb816e2 100644
--- a/src/lua/init.c
+++ b/src/lua/init.c
@@ -279,6 +279,7 @@ static const char *lua_modules[] = {
/* Make it first to affect load of all other modules */
"strict", strict_lua,
"compat", compat_lua,
+ "internal.utils", utils_lua,
"fun", fun_lua,
"debug", debug_lua,
"tarantool", init_lua,
@@ -303,7 +304,6 @@ static const char *lua_modules[] = {
"tap", tap_lua,
"help.en_US", help_en_US_lua,
"help", help_lua,
- "internal.utils", utils_lua,
"internal.argparse", argparse_lua,
"internal.trigger", trigger_lua,
"pwd", pwd_lua,
diff --git a/src/lua/utils.lua b/src/lua/utils.lua
index 53797e2834..b69d64ef45 100644
--- a/src/lua/utils.lua
+++ b/src/lua/utils.lua
@@ -1,3 +1,6 @@
+local ffi = require('ffi')
+local tarantool = require('tarantool')
+
local utils = {}
-- Same as type(), but returns 'number' if 'param' is
@@ -111,4 +114,34 @@ function utils.update_param_table(table, defaults)
return new_table
end
+ffi.cdef[[
+void
+__asan_poison_memory_region(void const volatile *addr, size_t size);
+void
+__asan_unpoison_memory_region(void const volatile *addr, size_t size);
+int
+__asan_address_is_poisoned(void const volatile *addr);
+]]
+
+if tarantool.build.asan then
+ utils.poison_memory_region = function(start, size)
+ ffi.C.__asan_poison_memory_region(start, size)
+ end
+ utils.unpoison_memory_region = function(start, size)
+ ffi.C.__asan_unpoison_memory_region(start, size)
+ end
+ utils.memory_region_is_poisoned = function(start, size)
+ for i = 0, size - 1 do
+ if ffi.C.__asan_address_is_poisoned(start + i) == 0 then
+ return false
+ end
+ end
+ return true
+ end
+else
+ utils.poison_memory_region = function() end
+ utils.unpoison_memory_region = function() end
+ utils.memory_region_is_poisoned = function() return false end
+end
+
return utils
diff --git a/test/app-luatest/buffer_test.lua b/test/app-luatest/buffer_test.lua
new file mode 100644
index 0000000000..46a32e2ff0
--- /dev/null
+++ b/test/app-luatest/buffer_test.lua
@@ -0,0 +1,81 @@
+local tarantool = require('tarantool')
+local buffer = require('buffer')
+local utils = require('internal.utils')
+local ffi = require('ffi')
+local t = require('luatest')
+
+local g = t.group()
+
+g.test_poison = function()
+ t.skip_if(not tarantool.build.asan, 'only make sense with ASAN')
+
+ local buf = buffer.ibuf()
+ local is_poisoned = function(buf, ptr)
+ t.assert(utils.memory_region_is_poisoned(ptr, buf:unused()))
+ end
+
+ -- Test poison on allocation. --
+ local ptr = buf:alloc(99)
+ t.assert(ptr ~= nil)
+ ffi.fill(ptr, 99)
+ is_poisoned(buf, ptr + 99)
+
+ t.assert(buf:unused() > 133)
+ ptr = buf:alloc(133)
+ t.assert(ptr ~= nil)
+ ffi.fill(ptr, 133)
+ is_poisoned(buf, ptr + 133)
+
+ local size = buf:unused() + 77
+ ptr = buf:alloc(size)
+ t.assert(ptr ~= nil)
+ ffi.fill(ptr, size)
+ t.assert_gt(buf:unused(), 0)
+ is_poisoned(buf, ptr + size)
+
+ -- Test poison after reset. --
+ buf:reset()
+ ptr = buf:alloc(0)
+ is_poisoned(buf, ptr)
+
+ -- Test poison on reserve. --
+ ptr = buf:reserve(777)
+ t.assert(ptr ~= nil)
+ t.assert_ge(buf:unused(), 777)
+ ffi.fill(ptr, buf:unused())
+ ptr = buf:alloc(333)
+ t.assert(ptr ~= nil)
+ ffi.fill(ptr, 333)
+ is_poisoned(buf, ptr + 333)
+
+ t.assert_gt(buf:unused(), 888)
+ ptr = buf:reserve(333)
+ t.assert(ptr ~= nil)
+ t.assert_ge(buf:unused(), 333)
+ ffi.fill(ptr, buf:unused())
+ ptr = buf:alloc(888)
+ t.assert(ptr ~= nil)
+ ffi.fill(ptr, 888)
+ is_poisoned(buf, ptr + 888)
+
+ size = buf:unused() + 133;
+ ptr = buf:reserve(size)
+ t.assert(ptr ~= nil)
+ t.assert_ge(buf:unused(), size)
+ ffi.fill(ptr, buf:unused())
+ t.assert_gt(buf:unused(), 0)
+ ptr = buf:alloc(size)
+ t.assert(ptr ~= nil)
+ t.assert_gt(buf:unused(), 0)
+ is_poisoned(buf, ptr + size)
+
+ size = buf:unused(buf) + 221;
+ buf:read(337)
+ ptr = buf:reserve(size)
+ t.assert(ptr ~= nil)
+ t.assert_ge(buf:unused(), size)
+ ffi.fill(ptr, buf:unused())
+ ptr = buf:alloc(size)
+ t.assert_gt(buf:unused(), 0)
+ is_poisoned(buf, ptr + size)
+end
diff --git a/test/box-tap/merger.test.lua b/test/box-tap/merger.test.lua
index 2d313daa3a..f22aaa5c5b 100755
--- a/test/box-tap/merger.test.lua
+++ b/test/box-tap/merger.test.lua
@@ -44,10 +44,10 @@ local function truncated_msgpack_buffer(data, trunc)
local len = data:len()
local buf = buffer.ibuf()
-- Ensure we have enough buffer to write len + trunc bytes.
- buf:reserve(len + trunc)
- local p = buf:alloc(len)
+ local p = buf:reserve(len + trunc)
-- Ensure len bytes follows with trunc zero bytes.
ffi.copy(p, data .. string.rep('\0', trunc), len + trunc)
+ buf:alloc(len)
return buf
end
--
GitLab