[Tarantool-patches] [PATCH luajit 1/2] Prevent sanitizer warning in snap_restoredata().
Sergey Bronnikov
sergeyb at tarantool.org
Thu Jul 4 10:58:40 MSK 2024
Hi, Sergey,
thanks for the patch!
Please add a message "Also, it removes all related UBSAN suppressions,
since they are fixed."
to commit message, like you did for the second patch.
LGTM
On 25.06.2024 18:54, Sergey Kaplun wrote:
> From: Mike Pall <mike>
>
> Thanks to Sergey Kaplun.
>
> (cherry picked from commit 4a22050df9e76a28ef904382e4b4c69578973cd5)
>
> When saving FPR registers during while from a trace and restoring
> data from a snapshot, UB sanitizer produces the following warning:
> | lj_snap.c:804:32: runtime error: index 23 out of bounds for type 'intptr_t [16]'
>
> due to indexing `ex->gpr` with a fpr register, whose number is >=
> `RID_MAX_GPR`. The situation itself is harmless since this is read from
> `spill[256]` array and is rewritten in the next if branch.
>
> This patch fixes the out-of-bounds access to read from `ex->gpr` only
> conditionally.
>
> Sergey Kaplun:
> * added the description and the test for the problem
>
> Part of tarantool/tarantool#9924
> Relates to tarantool/tarantool#8473
> ---
> src/lj_snap.c | 13 +++------
> ...93-out-of-bounds-snap-restoredata.test.lua | 28 +++++++++++++++++++
> 2 files changed, 32 insertions(+), 9 deletions(-)
> create mode 100644 test/tarantool-tests/lj-1193-out-of-bounds-snap-restoredata.test.lua
>
> diff --git a/src/lj_snap.c b/src/lj_snap.c
> index 7dc4fe35..8a33dc22 100644
> --- a/src/lj_snap.c
> +++ b/src/lj_snap.c
> @@ -756,13 +756,6 @@ static void snap_restoreval(jit_State *J, GCtrace *T, ExitState *ex,
> }
>
> #if LJ_HASFFI
> -# if LUAJIT_USE_UBSAN
> -/* Seehttps://github.com/LuaJIT/LuaJIT/issues/1193. */
> -static void snap_restoredata(jit_State *J, GCtrace *T, ExitState *ex,
> - SnapNo snapno, BloomFilter rfilt,
> - IRRef ref, void *dst, CTSize sz)
> - __attribute__((no_sanitize("bounds")));
> -# endif
> /* Restore raw data from the trace exit state. */
> static void snap_restoredata(jit_State *J, GCtrace *T, ExitState *ex,
> SnapNo snapno, BloomFilter rfilt,
> @@ -801,7 +794,6 @@ static void snap_restoredata(jit_State *J, GCtrace *T, ExitState *ex,
> *(lua_Number *)dst = (lua_Number)*(int32_t *)dst;
> return;
> }
> - src = (int32_t *)&ex->gpr[r-RID_MIN_GPR];
> #if !LJ_SOFTFP
> if (r >= RID_MAX_GPR) {
> src = (int32_t *)&ex->fpr[r-RID_MIN_FPR];
> @@ -815,7 +807,10 @@ static void snap_restoredata(jit_State *J, GCtrace *T, ExitState *ex,
> #endif
> } else
> #endif
> - if (LJ_64 && LJ_BE && sz == 4) src++;
> + {
> + src = (int32_t *)&ex->gpr[r-RID_MIN_GPR];
> + if (LJ_64 && LJ_BE && sz == 4) src++;
> + }
> }
> }
> lj_assertJ(sz == 1 || sz == 2 || sz == 4 || sz == 8,
> diff --git a/test/tarantool-tests/lj-1193-out-of-bounds-snap-restoredata.test.lua b/test/tarantool-tests/lj-1193-out-of-bounds-snap-restoredata.test.lua
> new file mode 100644
> index 00000000..6c5fc3f6
> --- /dev/null
> +++ b/test/tarantool-tests/lj-1193-out-of-bounds-snap-restoredata.test.lua
> @@ -0,0 +1,28 @@
> +local tap = require('tap')
> +
> +-- Test file to demonstrate LuaJIT's out-of-bounds access during
> +-- the saving of registers content in `snap_restoredata()`.
> +-- See also:https://github.com/LuaJIT/LuaJIT/issues/1193.
> +
> +local test = tap.test('lj-1193-out-of-bounds-snap-restoredata'):skipcond({
> + ['Test requires JIT enabled'] = not jit.status(),
> +})
> +
> +local ffi = require('ffi')
> +
> +test:plan(1)
> +
> +local double_type = ffi.typeof('double')
> +
> +jit.opt.start('hotloop=1')
> +local x = 1LL
> +for _ = 1, 4 do
> + -- `x` is saved in the fpr register and will be restored in the
> + -- `ex->fpr` during exit from the snapshot. But out-of-bounds
> + -- access is happening due to indexing `ex->gpr` occasionally.
> + x = double_type(x + 1)
> +end
> +
> +test:ok(true, 'no out-of-bounds failure')
> +
> +test:done(true)
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.tarantool.org/pipermail/tarantool-patches/attachments/20240704/d7427497/attachment.htm>
More information about the Tarantool-patches
mailing list