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 E5E656F44C9; Thu, 30 Nov 2023 10:30:00 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org E5E656F44C9 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1701329401; bh=vFBR0I06um7lvbeV8w9o+iYMBrBuyhWA4Rd6YWATkZw=; 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=dCbOy4ek7GasI2hUjrY+3QxbRGXLQPeWVKmvRf3oR37MjoRLXrPIdMuBBCpo0fTA0 3kmD9Wj8J7K8ZqtS8OqcJMCYC2wBijz43s/p6+F0BHwtf29tWv+kTSAwrwxYmzMFjB OKWbsfaHeLBjkVvaUObMhkY+cc64Evre2JlxVyps= Received: from smtp45.i.mail.ru (smtp45.i.mail.ru [95.163.41.83]) (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 02C1B6D4662 for ; Thu, 30 Nov 2023 10:29:59 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 02C1B6D4662 Received: by smtp45.i.mail.ru with esmtpa (envelope-from ) id 1r8bUk-00Az8Y-0f; Thu, 30 Nov 2023 10:29:58 +0300 Date: Thu, 30 Nov 2023 10:25:28 +0300 To: Sergey Bronnikov Message-ID: References: <9faa1bde-fd48-4dd2-8760-25c308b743a8@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <9faa1bde-fd48-4dd2-8760-25c308b743a8@tarantool.org> X-Mailru-Src: smtp X-4EC0790: 10 X-7564579A: 646B95376F6C166E X-77F55803: 4F1203BC0FB41BD9D2A6479154BC7F41EFB1454F5F9907FCF3615B3156E5803B182A05F5380850406D0AC9A35358AAF7E093B2F79ACD7C4FA2015EC2335EF90CD461F05B600BFFEF X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE75644E22E05AA81AEB287FD4696A6DC2FA8DF7F3B2552694A4E2F5AFA99E116B42401471946AA11AF23F8577A6DFFEA7C90AE7DF0B40174E18F08D7030A58E5AD1A62830130A00468AEEEE3FBA3A834EE7353EFBB55337566B720CBEDDCA346D4540F508EEA1C3A8179FC11BA9D5907B4A471835C12D1D9774AD6D5ED66289B5278DA827A17800CE7328B01A8D746D8839FA2833FD35BB23D2EF20D2F80756B5F868A13BD56FB6657A471835C12D1D977725E5C173C3A84C3E478A468B35FE767117882F4460429728AD0CFFFB425014E868A13BD56FB6657E2021AF6380DFAD1A18204E546F3947CB861051D4BA689FC2E808ACE2090B5E1725E5C173C3A84C3C5EA940A35A165FF2DBA43225CD8A89F616AD31D0D18CD5C35872C767BF85DA2F004C90652538430E4A6367B16DE6309 X-C1DE0DAB: 0D63561A33F958A54F58BDFD8000A9B1C55EF7FEF30B4606F53627F27C6F9E9FF87CCE6106E1FC07E67D4AC08A07B9B065B78C30F681404DCB5012B2E24CD356 X-C8649E89: 1C3962B70DF3F0ADBF74143AD284FC7177DD89D51EBB7742424CF958EAFF5D571004E42C50DC4CA955A7F0CF078B5EC49A30900B95165D34B3611847B8BC2D0B8180D769FD3F7B3D9745EFAB2B1376FE0CB9F64D33B18F256DAEBE6BC3B6FEBB1D7E09C32AA3244CDFDE5F1221A27EA62F03099B6888E4FF1DD47778AE04E04D85A42E4C463514DC5DA084F8E80FEBD3202CD0F03380D9577A83BD0C44CE203720ABEDE4BBDD9CDD X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojLZJja4YqbLQXuP0KWelqgw== X-DA7885C5: 347CA43641F7246A90A83B674A3269C7BABCA753B2D0D4ED906BDD9F81C81BCE262E2D401490A4A0DB037EFA58388B346E8BC1A9835FDE71 X-Mailru-Sender: 689FA8AB762F7393590D8C940224AE331D925A247B6E5BA1BA4C34533D4E65EA0FBE9A32752B8C9C2AA642CC12EC09F1FB559BB5D741EB962F61BD320559CF1EFD657A8799238ED55FEEDEB644C299C0ED14614B50AE0675 X-Mras: Ok Subject: Re: [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" Hi, Sergey! Thanks for the review! Please, consider my answers below. On 29.11.23, Sergey Bronnikov wrote: > Hello, Sergey > > thanks for the patch! See comments below > > On 10/23/23 12:22, Sergey Kaplun wrote: > > 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. > > failed to reproduce a bug with reverted fix and LuaJIT built with a flag: > > $ CMAKE_C_FLAGS=-DLUAJIT_CTYPE_CHECK_ANCHOR > CFLAGS=-DLUAJIT_CTYPE_CHECK_ANCHOR cmake -S . -B build > -DCMAKE_BUILD_TYPE=Debug -DLUA_USE_ASSERT=ON -DLUAJIT_USE_ASAN=ON I suppose you forgot the -DLUAJIT_USE_SYSMALLOC=ON -DLUAJIT_ENABLE_GC64=ON flags. The same is valid for the previous patch. > $ ./build/src/luajit > test/tarantool-tests/lj-920-fix-dangling-reference-to-ctype.test.lua > TAP version 13 > 1..1 > ok - no heap-use-after-free in recff_cdata_arith > > > > > 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 > > > > +++ 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, > > Let's declare _TARANTOOL as global in .luacheckrc instead of suppressing > warning inline every time. > > About 7 tests suppress it. I suppose it is a good thing to change, but in a separate patchset. Ignoring for now, you may create a ticket for refactoring if you want (to prevent falling off the edge of the earth). > > > +}) > > +test:done(true) -- Best regards, Sergey Kaplun