From: Sergey Kaplun via Tarantool-patches <tarantool-patches@dev.tarantool.org> To: Maxim Kokryashkin <m.kokryashkin@tarantool.org>, Sergey Bronnikov <sergeyb@tarantool.org> Cc: tarantool-patches@dev.tarantool.org Subject: [Tarantool-patches] [PATCH luajit] Fix unsinking of IR_FSTORE for NULL metatable. Date: Wed, 31 Jan 2024 15:18:59 +0300 [thread overview] Message-ID: <20240131121859.18071-1-skaplun@tarantool.org> (raw) From: Mike Pall <mike> Reported by pwnhacker0x18. (cherry picked from commit 85b4fed0b0353dd78c8c875c2f562d522a2b310f) The `FSTORE` restoring of a sunk table from a snapshot for `IRFL_TAB_META` misses the case when the second argument of `setmetatable()` is `nil` (so, the `FSTORE` second operand is `NULL`). This may lead to the corresponding assertion failure in the `snap_replay_const()` or the crash. This patch handles the aforementioned case. Sergey Kaplun: * added the description and the test for the problem Part of tarantool/tarantool#9595 --- Branch: https://github.com/tarantool/luajit/tree/skaplun/lj-1147-fstore-null-meta Tarantool PR: https://github.com/tarantool/tarantool/pull/9635 Related issues: * https://github.com/tarantool/tarantool/issues/9595 * https://github.com/LuaJIT/LuaJIT/issues/1147 src/lj_snap.c | 11 +++-- .../lj-1147-fstore-null-meta.test.lua | 41 +++++++++++++++++++ 2 files changed, 49 insertions(+), 3 deletions(-) create mode 100644 test/tarantool-tests/lj-1147-fstore-null-meta.test.lua diff --git a/src/lj_snap.c b/src/lj_snap.c index 73e18e69..26352080 100644 --- a/src/lj_snap.c +++ b/src/lj_snap.c @@ -414,6 +414,7 @@ static TRef snap_replay_const(jit_State *J, IRIns *ir) case IR_KNUM: case IR_KINT64: return lj_ir_k64(J, (IROp)ir->o, ir_k64(ir)->u64); case IR_KPTR: return lj_ir_kptr(J, ir_kptr(ir)); /* Continuation. */ + case IR_KNULL: return lj_ir_knull(J, irt_type(ir->t)); default: lj_assertJ(0, "bad IR constant op %d", ir->o); return TREF_NIL; } } @@ -857,9 +858,13 @@ static void snap_unsink(jit_State *J, GCtrace *T, ExitState *ex, if (irk->o == IR_FREF) { switch (irk->op2) { case IRFL_TAB_META: - snap_restoreval(J, T, ex, snapno, rfilt, irs->op2, &tmp); - /* NOBARRIER: The table is new (marked white). */ - setgcref(t->metatable, obj2gco(tabV(&tmp))); + if (T->ir[irs->op2].o == IR_KNULL) { + setgcrefnull(t->metatable); + } else { + snap_restoreval(J, T, ex, snapno, rfilt, irs->op2, &tmp); + /* NOBARRIER: The table is new (marked white). */ + setgcref(t->metatable, obj2gco(tabV(&tmp))); + } break; case IRFL_TAB_NOMM: /* Negative metamethod cache invalidated by lj_tab_set() below. */ diff --git a/test/tarantool-tests/lj-1147-fstore-null-meta.test.lua b/test/tarantool-tests/lj-1147-fstore-null-meta.test.lua new file mode 100644 index 00000000..bdf60a26 --- /dev/null +++ b/test/tarantool-tests/lj-1147-fstore-null-meta.test.lua @@ -0,0 +1,41 @@ +local tap = require('tap') + +-- Test file to demonstrate LuaJIT's incorrect restoration of a +-- table from a snapshot when the `setmetatable()` gets `nil` as +-- the second argument. +-- See also: https://github.com/LuaJIT/LuaJIT/issues/1147. + +local test = tap.test('lj-1147-fstore-null-meta'):skipcond({ + ['Test requires JIT enabled'] = not jit.status(), +}) + +test:plan(2) + +jit.opt.start('hotloop=1') + +local counter = 0 +local tab +-- XXX: The loop is limited to 3 iterations to compile a trace and +-- start to execute it. Use the loop format to see +-- the side effects on the restoration from the snapshot. +local LOOP_LIMIT = 2 +while true do + counter = counter + 1 + -- Use counter for the content check. + tab = {counter} + -- This emits the following IRs necessary for the assertion + -- failure: + -- | 0003 }+ tab TNEW #3 #0 + -- | ... + -- | 0015 p64 FREF 0003 tab.meta + -- | 0016 } tab FSTORE 0015 NULL + setmetatable(tab, nil) + -- Emit exit here to be sure that the table will be restored + -- from the snapshot. + if counter > LOOP_LIMIT then break end +end + +test:is(tab[1], LOOP_LIMIT + 1, 'correct table content') +test:ok(debug.getmetatable(tab) == nil, 'no metatable on the restored table') + +test:done(true) -- 2.43.0
next reply other threads:[~2024-01-31 12:22 UTC|newest] Thread overview: 4+ messages / expand[flat|nested] mbox.gz Atom feed top 2024-01-31 12:18 Sergey Kaplun via Tarantool-patches [this message] 2024-02-01 9:42 ` Maxim Kokryashkin via Tarantool-patches 2024-02-01 10:48 ` Sergey Bronnikov via Tarantool-patches 2024-02-15 13:46 ` Igor Munkin 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=20240131121859.18071-1-skaplun@tarantool.org \ --to=tarantool-patches@dev.tarantool.org \ --cc=m.kokryashkin@tarantool.org \ --cc=sergeyb@tarantool.org \ --cc=skaplun@tarantool.org \ --subject='Re: [Tarantool-patches] [PATCH luajit] Fix unsinking of IR_FSTORE for NULL metatable.' \ /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