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 7AAC467396E; Mon, 23 Oct 2023 12:29:08 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 7AAC467396E DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1698053348; bh=soBjQY/hi6fnzQ+jNcBC4h4V3JolY18WA2mTLZYZu7I=; h=To:Date:In-Reply-To:References:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=qh+ktJb0K4ddCRVuJ9fMQgQzI/v5JKi/e6aClJ3n4ku/itsEEBH10DhOwQcFLt7vr JNWhbX4OnE3v7md49HN9qBBEu7sCUT9eAAP2mT9evuOUmkB8zmHpaSf9PFmIIwpiOB TBMYWb0goGrD1W4W8BCf8tcvAQPMMhh5lRH8+Afk= Received: from smtpng1.i.mail.ru (smtpng1.i.mail.ru [94.100.181.251]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by dev.tarantool.org (Postfix) with ESMTPS id A23BB673969 for ; Mon, 23 Oct 2023 12:26:40 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org A23BB673969 Received: by smtpng1.m.smailru.net with esmtpa (envelope-from ) id 1qurCp-0007cl-Iv; Mon, 23 Oct 2023 12:26:40 +0300 To: Maxim Kokryashkin , Sergey Bronnikov Date: Mon, 23 Oct 2023 12:22:05 +0300 Message-ID: X-Mailer: git-send-email 2.42.0 In-Reply-To: References: MIME-Version: 1.0 Content-Transfer-Encoding: 8bit X-Mailru-Src: smtp X-4EC0790: 10 X-7564579A: B8F34718100C35BD X-77F55803: 4F1203BC0FB41BD9C197A170B57C5E43424B4CF9DFD4ED6F3F9F04D90C6DD02600894C459B0CD1B996B4E8A55734C30A60B56728870CD69E08F89D8EC0FEE90FFD50102C7C5B78F1 X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE7A1DB0B089319D380EA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F790063765D69633A07E2F0F8638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D8A1FAD9BE9C56B7A7415B33E298280E6B117882F4460429724CE54428C33FAD305F5C1EE8F4F765FCF80095D1E57F4578A471835C12D1D9774AD6D5ED66289B52BA9C0B312567BB23117882F446042972877693876707352033AC447995A7AD18BDFBBEFFF4125B51D2E47CDBA5A96583BA9C0B312567BB231DD303D21008E29813377AFFFEAFD269176DF2183F8FC7C0E9CA1980FA7BB05068655334FD4449CB33AC447995A7AD1857739F23D657EF2BD5E8D9A59859A8B679E85A9FDE02C04A75ECD9A6C639B01B4E70A05D1297E1BBCB5012B2E24CD356 X-C1DE0DAB: 0D63561A33F958A5423B5636100D78C4F5A1C08BD931AD5FB7CB26BF0058BBD4F87CCE6106E1FC07E67D4AC08A07B9B0B4B51A2BAB7FBE05C79554A2A72441328621D336A7BC284946AD531847A6065A17B107DEF921CE79BDAD6C7F3747799A X-C8649E89: 1C3962B70DF3F0ADE00A9FD3E00BEEDF3FED46C3ACD6F73ED3581295AF09D3DF87807E0823442EA2ED31085941D9CD0AF7F820E7B07EA4CF48157ACF3214FD4F3383653D16EC103FDDEED2FD25C057A58BB81E60B05E40B02219C82DFA2AC3F2A8B50B9382ADB12A870AA7F1E2C05711E9FECCEEFE18C50EA74DFFEFA5DC0E7F02C26D483E81D6BE5EF9655DD6DEA7D65774BB76CC95456EEC5B5AD62611EEC62B5AFB4261A09AF0 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojqlVu258LHAF8SbA+vZUExw== X-DA7885C5: 94FAFE566B8587DC7C62A0077695275C1D65D0F152511B246D0F10A81E287421262E2D401490A4A0DB037EFA58388B346E8BC1A9835FDE71 X-Mailru-Sender: 689FA8AB762F73930F533AC2B33E986BAEC3B131A73B89536A3967B4FEE703970FBE9A32752B8C9C2AA642CC12EC09F1FB559BB5D741EB962F61BD320559CF1EFD657A8799238ED55FEEDEB644C299C0ED14614B50AE0675 X-Mras: Ok Subject: [Tarantool-patches] [PATCH luajit 5/6] FFI: Fix dangling reference to CType. Improve checks. 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 Kaplun via Tarantool-patches Reply-To: Sergey Kaplun Cc: tarantool-patches@dev.tarantool.org Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" From: Mike Pall Reported by elmknot. (cherry-picked from commit cc96ab9d513582703f8663a8775a935b56db32b7) During the recording of an arithmetic operation with a cdata function object and some cdata value in `recff_cdata_arith()`, reallocation of `cts->tab` in `lj_ctype_intern()` may occur. In that case, the reference to the first `CType` object (`s[0]`) becomes invalid. This patch saves the `CTypeID` of this object and gets its `CType` again after possible reallocation. Also, this commit fills `cts->tab` memory with zeros before being freed when `-DLUAJIT_CTYPE_CHECK_ANCHOR` is defined. It helps to observe assertion failures in case this memory is used after free. Sergey Kaplun: * added the description and the test for the problem Part of tarantool/tarantool#9145 --- src/lj_crecord.c | 4 + src/lj_ctype.c | 12 +++ ...0-fix-dangling-reference-to-ctype.test.lua | 77 +++++++++++++++++++ 3 files changed, 93 insertions(+) create mode 100644 test/tarantool-tests/lj-920-fix-dangling-reference-to-ctype.test.lua diff --git a/src/lj_crecord.c b/src/lj_crecord.c index 10d1dc70..e17e512f 100644 --- a/src/lj_crecord.c +++ b/src/lj_crecord.c @@ -1502,9 +1502,13 @@ void LJ_FASTCALL recff_cdata_arith(jit_State *J, RecordFFData *rd) if (ctype_isenum(ct->info)) ct = ctype_child(cts, ct); goto ok; } else if (ctype_isfunc(ct->info)) { + CTypeID id0 = i ? ctype_typeid(cts, s[0]) : 0; tr = emitir(IRT(IR_FLOAD, IRT_PTR), tr, IRFL_CDATA_PTR); ct = ctype_get(cts, lj_ctype_intern(cts, CTINFO(CT_PTR, CTALIGN_PTR|id), CTSIZE_PTR)); + if (i) { + s[0] = ctype_get(cts, id0); /* cts->tab may have been reallocated. */ + } goto ok; } else { tr = emitir(IRT(IR_ADD, IRT_PTR), tr, lj_ir_kintp(J, sizeof(GCcdata))); diff --git a/src/lj_ctype.c b/src/lj_ctype.c index a42e3d60..0874fa61 100644 --- a/src/lj_ctype.c +++ b/src/lj_ctype.c @@ -191,8 +191,20 @@ CTypeID lj_ctype_intern(CTState *cts, CTInfo info, CTSize size) } id = cts->top; if (LJ_UNLIKELY(id >= cts->sizetab)) { +#ifdef LUAJIT_CTYPE_CHECK_ANCHOR + CType *ct; +#endif if (id >= CTID_MAX) lj_err_msg(cts->L, LJ_ERR_TABOV); +#ifdef LUAJIT_CTYPE_CHECK_ANCHOR + ct = lj_mem_newvec(cts->L, id+1, CType); + memcpy(ct, cts->tab, id*sizeof(CType)); + memset(cts->tab, 0, id*sizeof(CType)); + lj_mem_freevec(cts->g, cts->tab, cts->sizetab, CType); + cts->tab = ct; + cts->sizetab = id+1; +#else lj_mem_growvec(cts->L, cts->tab, cts->sizetab, CTID_MAX, CType); +#endif } cts->top = id+1; cts->tab[id].info = info; diff --git a/test/tarantool-tests/lj-920-fix-dangling-reference-to-ctype.test.lua b/test/tarantool-tests/lj-920-fix-dangling-reference-to-ctype.test.lua new file mode 100644 index 00000000..a7e35888 --- /dev/null +++ b/test/tarantool-tests/lj-920-fix-dangling-reference-to-ctype.test.lua @@ -0,0 +1,77 @@ +local tap = require('tap') +local ffi = require('ffi') +local test = tap.test('lj-920-fix-dangling-reference-to-ctype'):skipcond({ + ['Test requires JIT enabled'] = not jit.status(), + -- luacheck: no global + ['Impossible to predict the value of cts->top'] = _TARANTOOL, +}) + +test:plan(1) + +-- This test demonstrates LuaJIT's incorrect behaviour when the +-- reallocation of `cts->tab` strikes during the recording of the +-- cdata metamethod arithmetic. +-- The test fails under ASAN. + +-- XXX: Just some C functions to be casted. There is no need to +-- declare their prototypes correctly. +ffi.cdef[[ + int malloc(void); + int fprintf(void); + int printf(void); + int memset(void); + int memcpy(void); + int memmove(void); + int getppid(void); +]] + +local cfunc_type = ffi.metatype(ffi.typeof('struct {int a;}'), { + -- Just some metatable with reloaded arithmetic operator. + __add = function(o1, _) return o1 end +}) +-- Just some cdata with metamethod. +local test_value = cfunc_type(1) + +-- XXX: structure to set `cts->top` to 112. +local _ = ffi.new('struct {int a; long b; float c; double d;}', 0) + +-- Anchor table to prevent cdata objects from being collected. +local anchor = {} +-- Each call to this function grows `cts->top` by 3. +local function save_new_func(func) + anchor[#anchor + 1] = ffi.cast('void (*)(void)', func) +end + +save_new_func(ffi.C.fprintf) -- `cts->top` = 112 +save_new_func(ffi.C.printf) -- `cts->top` = 115 +save_new_func(ffi.C.memset) -- `cts->top` = 118 +save_new_func(ffi.C.memcpy) -- `cts->top` = 121 +save_new_func(ffi.C.malloc) -- `cts->top` = 124 + +-- Assertions to check the `cts->top` value and step between +-- calls. +assert(ffi.typeinfo(124), 'cts->top >= 124') +assert(not ffi.typeinfo(125), 'cts->top < 125') + +save_new_func(ffi.C.memmove) -- `cts->top` = 127 + +jit.opt.start('hotloop=1') + +-- Just some function to record trace and cause reallocation. +local function recorded_func(func_arg) + local res = test_value + func_arg + return res +end +recorded_func(ffi.C.malloc) + +assert(ffi.typeinfo(127), 'cts->top >= 127') +assert(not ffi.typeinfo(128), 'cts->top < 128') + +-- Last call to grow `cts->top` up to 129, so this causes +-- `cts->tab` reallocation. Need to use different functions as +-- an argument. +recorded_func(ffi.C.getppid) + +test:ok(true, 'no heap-use-after-free in recff_cdata_arith') + +test:done(true) -- 2.42.0