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 792C565341F; Wed, 25 Oct 2023 11:07:11 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 792C565341F DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1698221231; bh=nHQz501LcvBTnvl4VvxGoREo5/OwHuq1Y81sm8005u4=; h=Date:To:References:In-Reply-To:Subject:List-Id:List-Unsubscribe: List-Archive:List-Post:List-Help:List-Subscribe:From:Reply-To:Cc: From; b=DrYblDXSRunYTUm7Sz4fbr+pnA6hfqGhiGLKlxVPcmYsy9XLyHIUuez4hdS+xmQWU js9pJAXfNz5xFhSVp7ODqo7MtYQpmt23Z1OM84Sm+S7JD+X5DzpQPg98Q5pZWSoozz 9WFYsqZ+XdNEL5JIIfCiXyoaWSX/yZFMwRTEkCy8= Received: from smtp42.i.mail.ru (smtp42.i.mail.ru [95.163.41.65]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 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 D96FD65341F for ; Wed, 25 Oct 2023 11:07:09 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org D96FD65341F Received: by smtp42.i.mail.ru with esmtpa (envelope-from ) id 1qvYuy-00H9HW-3A; Wed, 25 Oct 2023 11:07:09 +0300 Date: Wed, 25 Oct 2023 11:07:08 +0300 To: Sergey Kaplun Message-ID: References: <33a9a2fc1efb801bfd5b8101be16755f2b394293.1698049570.git.skaplun@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <33a9a2fc1efb801bfd5b8101be16755f2b394293.1698049570.git.skaplun@tarantool.org> X-Mailru-Src: smtp X-4EC0790: 10 X-7564579A: 646B95376F6C166E X-77F55803: 4F1203BC0FB41BD9315D6E1B1B5C3D6DAB173C7F323F97EC4594442A252DA51100894C459B0CD1B9FE5E580A9DCE798E768E5D1D6487CD1C5C2DBC97BDB523BF6F8F9941A8C2109B X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE75AD53DF1D86BACA3EA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F79006379FED03E15F715FAA8638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D8D601F9F779BD1559C97ED09D6B9430A9117882F4460429724CE54428C33FAD305F5C1EE8F4F765FCECADA55FE5B58BB7A471835C12D1D9774AD6D5ED66289B52BA9C0B312567BB23117882F446042972877693876707352033AC447995A7AD18E5D25F19253116ADD2E47CDBA5A96583BA9C0B312567BB2376E601842F6C81A19E625A9149C048EE140C956E756FBB7A452896749CDDA0A6D8FC6C240DEA76429C9F4D5AE37F343AA9539A8B242431040A6AB1C7CE11FEE32A336C651863509103F1AB874ED89028C4224003CC836476E2F48590F00D11D6E2021AF6380DFAD1A18204E546F3947CA9FF340AA05FB58C2E808ACE2090B5E1725E5C173C3A84C3C5EA940A35A165FF2DBA43225CD8A89F83C798A30B85E16BCE5475246E174218B5C8C57E37DE458BEDA766A37F9254B7 X-C1DE0DAB: 0D63561A33F958A590F07ED7A6F95DC734C8ABBE31A04BD2935D602F1671D047F87CCE6106E1FC07E67D4AC08A07B9B0CF7CD7A0D5AA5F25CB5012B2E24CD356 X-C8649E89: 1C3962B70DF3F0ADE00A9FD3E00BEEDF3FED46C3ACD6F73ED3581295AF09D3DF87807E0823442EA2ED31085941D9CD0AF7F820E7B07EA4CFF0094606250FF9F2941A0ECD0AE1E9CB710E1D063DF6D1800F21F390D3BD5AAA16D5D591715857ECA789783558ABFE9CBE9006932AAD22D0133B554EA7706295E48CAC7CA610320002C26D483E81D6BE64ACE4A408B72B61B0CA6F94E606A667A52EF62A646584F811BD90D3D42C882D43082AE146A756F3 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2bioj7UiV2qa299p5/WNJVBWbzA== X-Mailru-Sender: 00097D31F91C944B15339A1EB722E503943ED481100E34E1C934C8AB0E2965ED19989B863DE37BB55EA65D4B9F24FE89AED07E233588217FA46513A1025A7217E7B7573491FBF26DB0DAF586E7D11B3E67EA787935ED9F1B X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH luajit 6/6] FFI: Fix dangling reference to CType in carith_checkarg(). 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: Maxim Kokryashkin via Tarantool-patches Reply-To: Maxim Kokryashkin Cc: tarantool-patches@dev.tarantool.org Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" Hi, Sergey! LGTM, after fixing the same comments, as for the previous patches. On Mon, Oct 23, 2023 at 12:22:06PM +0300, Sergey Kaplun wrote: > From: Mike Pall > > Reported by Sergey Kaplun. > > (cherry-picked from commit db944b2b56c86fcf133745976763604d96110285) > > During of an arithmetic operation with a cdata function object and some > cdata value in `carith_checkarg()`, reallocation of `cts->tab` in > `lj_ctype_intern()` may occur. In that case, the reference to the first > `CType` object (`ca->ct[0]`) becomes invalid. This patch saves the > `CTypeID` of this object and gets its `CType` again after possible > reallocation. > > Sergey Kaplun: > * added the description and the test for the problem > > Part of tarantool/tarantool#9145 > --- > src/lj_carith.c | 4 ++ > ...8-fix-dangling-reference-to-ctype.test.lua | 67 +++++++++++++++++++ > 2 files changed, 71 insertions(+) > create mode 100644 test/tarantool-tests/lj-1108-fix-dangling-reference-to-ctype.test.lua > > diff --git a/src/lj_carith.c b/src/lj_carith.c > index 4ae1e9ee..4e1d450a 100644 > --- a/src/lj_carith.c > +++ b/src/lj_carith.c > @@ -44,9 +44,13 @@ static int carith_checkarg(lua_State *L, CTState *cts, CDArith *ca) > p = (uint8_t *)cdata_getptr(p, ct->size); > if (ctype_isref(ct->info)) ct = ctype_rawchild(cts, ct); > } else if (ctype_isfunc(ct->info)) { > + CTypeID id0 = i ? ctype_typeid(cts, ca->ct[0]) : 0; > p = (uint8_t *)*(void **)p; > ct = ctype_get(cts, > lj_ctype_intern(cts, CTINFO(CT_PTR, CTALIGN_PTR|id), CTSIZE_PTR)); > + if (i) { /* cts->tab may have been reallocated. */ > + ca->ct[0] = ctype_get(cts, id0); > + } > } > if (ctype_isenum(ct->info)) ct = ctype_child(cts, ct); > ca->ct[i] = ct; > diff --git a/test/tarantool-tests/lj-1108-fix-dangling-reference-to-ctype.test.lua b/test/tarantool-tests/lj-1108-fix-dangling-reference-to-ctype.test.lua > new file mode 100644 > index 00000000..43a39886 > --- /dev/null > +++ b/test/tarantool-tests/lj-1108-fix-dangling-reference-to-ctype.test.lua > @@ -0,0 +1,67 @@ > +local tap = require('tap') > +local ffi = require('ffi') > +local test = tap.test('lj-1108-fix-dangling-reference-to-ctype'):skipcond({ > + -- 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 arithmetic cdata > +-- metamethod. > +-- 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 > + > +assert(ffi.typeinfo(127), 'cts->top >= 127') > +assert(not ffi.typeinfo(128), 'cts->top < 128') > + > +-- Just check cdata arith metamethod. The function argument should > +-- be the second because dangling reference is the CType of the > +-- first argumnent. > +_ = test_value + ffi.C.getppid > + > +test:ok(true, 'no heap-use-after-free in carith_checkarg') > + > +test:done(true) > -- > 2.42.0 >