From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from [87.239.111.99] (localhost [127.0.0.1]) by dev.tarantool.org (Postfix) with ESMTP id 133C5C7602A; Thu, 15 Aug 2024 11:20:58 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 133C5C7602A DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1723710058; bh=GwItoWASxYDhTn7NtkOeQl2vAg7+CM3lTRF+iaK7CYg=; h=To:Date:In-Reply-To:References:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To: From; b=VjkianC4Li9iuy8lrXwnarBa/ZwUtgyE5hggH68GaxSCYw9EN1E0tfHJBHeL0YQzp n9T8gEspYx8GwAp5tI4Avi8ixIfQ0CS/HMEAzu2NHEzkxRHxpFptf5V3SHakAqwyOY 9cLQNU/fwKgdXhT0ZMpsS95A4XaNmMWiPpDRMadI= Received: from mail-lj1-f174.google.com (mail-lj1-f174.google.com [209.85.208.174]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id E8CA1C76028 for ; Thu, 15 Aug 2024 11:20:56 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org E8CA1C76028 Received: by mail-lj1-f174.google.com with SMTP id 38308e7fff4ca-2f16767830dso7150941fa.0 for ; Thu, 15 Aug 2024 01:20:56 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1723710056; x=1724314856; h=content-transfer-encoding:mime-version:references:in-reply-to :message-id:date:subject:to:from:x-gm-message-state:from:to:cc :subject:date:message-id:reply-to; bh=bLDsN9E9clutVxoieyNsmw0MJYgEUhGKYhShLnlUEHc=; b=XlIESvOj7rmEnn+AE4NMImiI/LqF129s4XYkBRKrVSdFuoLt1j029habR2t4lXtuNK HxhUtp6oCou2NiHSxLye+6NT0PL9UWg/uYVE40rgKIVNmmHM11ozJyCm8mvIqJR9cEHM OLdxqhFudnrkuS/vGkkGE5/nybCbOvf4w37AgPTOtadZeWwvpshbRnRtbrN5Ek08+swE 67vMf+iYmXJZGRQ1fqspjUtSd7foxZt2/irqXGHrC9xu0270dvZFgkGwZBqE2SxXWDWl qr9OCaI/w94PtdM+Hk2QRhETcrP/59ecqSy+d7jNLpqsgOcKTE7ahxEo38vi7Xx9hvUi eOvQ== X-Gm-Message-State: AOJu0YwO3m7Ub94hkML41VMVwEJr5Hsh8/02/T+6ugjjTGZpCYlrKugO B/rH+X9ZlWyftzL2I9Vp3WY987b1c4fGbFCDrDPSQiW/NapgxJG3ts/H8BBV X-Google-Smtp-Source: AGHT+IGeMHasWIYD9ZFDzgEdCe0oNHWep4iuALccE0PziOOW6dxjg5StvFdDaq/lM0ZiObOPze2XDA== X-Received: by 2002:a2e:9087:0:b0:2ef:a504:65b8 with SMTP id 38308e7fff4ca-2f3aa1b727bmr30792181fa.13.1723710055604; Thu, 15 Aug 2024 01:20:55 -0700 (PDT) Received: from pony.mail.msk ([5.181.62.98]) by smtp.gmail.com with ESMTPSA id 5b1f17b1804b1-429e7df561dsm12358455e9.21.2024.08.15.01.20.54 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Thu, 15 Aug 2024 01:20:55 -0700 (PDT) To: tarantool-patches@dev.tarantool.org, Sergey Kaplun , Maxim Kokryashkin Date: Thu, 15 Aug 2024 11:20:31 +0300 Message-Id: <066c36f1d4a77ffefc020997222d873692bebcc5.1723708977.git.sergeyb@tarantool.org> X-Mailer: git-send-email 2.34.1 In-Reply-To: References: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Subject: [Tarantool-patches] [PATCH luajit 1/2][v2] FFI: Treat cdata finalizer table as a GC root. X-BeenThere: tarantool-patches@dev.tarantool.org X-Mailman-Version: 2.1.34 Precedence: list List-Id: Tarantool development patches List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , From: Sergey Bronnikov via Tarantool-patches Reply-To: Sergey Bronnikov Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" From: Mike Pall Thanks to Sergey Bronnikov. (cherry picked from commit dda1ac273ad946387088d91039a8ae319359903d) The finalizers table is created on initialization of the `ffi` module by calling the `ffi_finalizer()` routine in the `luaopen_ffi()`. `ffi.gc()` is referenced by Lua stack via the `ffi` library, and the finalizer table is anchored there as well. If there is no FFI module table anywhere to anchor the `ffi.gc` itself and the `lua_State` object is marked after the function `ffi.gc` is removed from it (since we stop the GC before chunk loading and start after), then the finalizer table isn't marked. Hence, after the atomic phase, the table is considered dead and collected. Since the table is collected, the usage of its nodes in the `lj_gc_finalize_cdata()` leads to heap-use-after-free. The patch fixes the problem partially by marking the finalizer table on the start of the GC cycle. The complete fix will be applied in the next patch by turning the finalizer table into the proper GC root. Sergey Bronnikov: * added the description and the tests for the problem Part of tarantool/tarantool#10199 --- src/lj_gc.c | 3 + .../lj-1168-unmarked-finalizer-tab.test.c | 76 +++++++++++++++++++ .../lj-1168-unmarked-finalizer-tab.test.lua | 18 +++++ 3 files changed, 97 insertions(+) create mode 100644 test/tarantool-c-tests/lj-1168-unmarked-finalizer-tab.test.c create mode 100644 test/tarantool-tests/lj-1168-unmarked-finalizer-tab.test.lua diff --git a/src/lj_gc.c b/src/lj_gc.c index 591862b3..42348a34 100644 --- a/src/lj_gc.c +++ b/src/lj_gc.c @@ -99,6 +99,9 @@ static void gc_mark_start(global_State *g) gc_markobj(g, tabref(mainthread(g)->env)); gc_marktv(g, &g->registrytv); gc_mark_gcroot(g); +#if LJ_HASFFI + if (ctype_ctsG(g)) gc_markobj(g, ctype_ctsG(g)->finalizer); +#endif g->gc.state = GCSpropagate; } diff --git a/test/tarantool-c-tests/lj-1168-unmarked-finalizer-tab.test.c b/test/tarantool-c-tests/lj-1168-unmarked-finalizer-tab.test.c new file mode 100644 index 00000000..d577b551 --- /dev/null +++ b/test/tarantool-c-tests/lj-1168-unmarked-finalizer-tab.test.c @@ -0,0 +1,76 @@ +#include "lua.h" +#include "lauxlib.h" + +#include "test.h" + +#define UNUSED(x) ((void)(x)) + +/* + * This test demonstrates LuaJIT's incorrect behaviour on + * loading Lua chunk with cdata numbers. + * See https://github.com/LuaJIT/LuaJIT/issues/1168 for details. + * + * The GC is driving forward during parsing of the Lua chunk + * (`test_chunk`). The chunk contains a single cdata object with + * a number. That leads to the opening of the FFI library + * on-demand during the parsing of this number. After the FFI + * library is open, `ffi.gc` has the finalizer table as its + * environment. But, there is no FFI module table anywhere to + * anchor the `ffi.gc` itself, and the `lua_State` object is + * marked after the function is removed from it. Hence, after the + * atomic phase, the table is considered dead and collected. Since + * the table is collected, the usage of its nodes in the + * `lj_gc_finalize_cdata` leads to heap-use-after-free. + */ + +const char buff[] = "return 1LL"; + +/* + * lua_close is a part of testcase, so testcase creates + * its own Lua state and closes it at the end. + */ +static int unmarked_finalizer_tab_gcstart(void *test_state) +{ + /* Shared Lua state is not needed. */ + UNUSED(test_state); + + /* Setup. */ + lua_State *L = luaL_newstate(); + + /* Set GC at the start. */ + lua_gc(L, LUA_GCCOLLECT, 0); + + /* Not trigger GC during `lua_openffi()`. */ + lua_gc(L, LUA_GCSTOP, 0); + + /* + * The terminating '\0' is considered by parser as part of + * the input, so we must chomp it. + */ + int res = luaL_loadbufferx(L, buff, sizeof(buff) - 1, + "test_chunk", "t"); + if (res != LUA_OK) { + test_comment("error loading Lua chunk: %s", + lua_tostring(L, -1)); + bail_out("error loading Lua chunk"); + } + + /* Finish GC cycle to collect the finalizer table. */ + while (!lua_gc(L, LUA_GCSTEP, -1)); + + /* Teardown. */ + lua_settop(L, 0); + lua_close(L); + + return TEST_EXIT_SUCCESS; +} + +int main(void) +{ + const struct test_unit tgroup[] = { + test_unit_def(unmarked_finalizer_tab_gcstart), + }; + const int test_result = test_run_group(tgroup, NULL); + + return test_result; +} diff --git a/test/tarantool-tests/lj-1168-unmarked-finalizer-tab.test.lua b/test/tarantool-tests/lj-1168-unmarked-finalizer-tab.test.lua new file mode 100644 index 00000000..4b49e9a1 --- /dev/null +++ b/test/tarantool-tests/lj-1168-unmarked-finalizer-tab.test.lua @@ -0,0 +1,18 @@ +local tap = require('tap') + +-- This test demonstrates LuaJIT's heap-use-after-free on +-- cleaning of resources during shutdown. The test simulates +-- "unloading" of the library, or removing some of its +-- functionality and then calls `collectgarbage`. +-- See https://github.com/LuaJIT/LuaJIT/issues/1168 for details. +local test = tap.test('lj-1168-unmarked-finalizer-tab') +test:plan(1) + +local ffi = require('ffi') + +ffi.gc = nil +collectgarbage() + +test:ok(true, 'no heap use after free') + +test:done(true) -- 2.34.1