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 8EC7C772E80; Mon, 12 Aug 2024 16:32:34 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 8EC7C772E80 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1723469554; bh=Emt9q2X4CihjtBNsZYjDKNnL+58YDncN2u8aEizJbfU=; h=Date:To:Cc:References:In-Reply-To:Subject:List-Id: List-Unsubscribe:List-Archive:List-Post:List-Help:List-Subscribe: From:Reply-To:From; b=pLGHHhxDBWZs27awljc9r47+JiTqYABhEvUEoUC63M1wIIKXpadIB9cFNSHURg7Yz QPtIX80Q+8GUXnhjs9PPFlo9F1/qUJOR7i2B7ebTA2jZQmzvlYnsvhcaW/1q37KUkH p0n7wYhQGSUOBiSqfBtWMStV5bSuJAAJ0cqj+i0s= Received: from smtp52.i.mail.ru (smtp52.i.mail.ru [95.163.41.88]) (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 9D278772E80 for ; Mon, 12 Aug 2024 16:32:33 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 9D278772E80 Received: by smtp52.i.mail.ru with esmtpa (envelope-from ) id 1sdVA0-0000000Aack-2SHV; Mon, 12 Aug 2024 16:32:33 +0300 Date: Mon, 12 Aug 2024 16:32:28 +0300 To: Sergey Bronnikov Cc: Sergey Bronnikov , tarantool-patches@dev.tarantool.org Message-ID: References: <48e2f924db3e2bdac356cefd9aae21e42556514b.1720521873.git.sergeyb@tarantool.org> <87f6aec7-c27b-4dde-8c2a-132eff77e048@tarantool.org> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: X-Mailru-Src: smtp X-4EC0790: 10 X-7564579A: 78E4E2B564C1792B X-77F55803: 4F1203BC0FB41BD90E4B1D041588DA6E99ADBE0B8B120BACE53B6AC40B44051F182A05F5380850400F30B91E309DBB6FC591814E25D11F9F988F912272586084DABD503DF037033BCEBD7663CDD6CE63 X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE7F65C230EDDCD559EEA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F7900637C904E3CF4B5CD3198638F802B75D45FF36EB9D2243A4F8B5A6FCA7DBDB1FC311F39EFFDF887939037866D6147AF826D8208E449164CADE50F28D115C2AC69BF552C493598E2A2220CC7F00164DA146DAFE8445B8C89999728AA50765F790063783E00425F71A4181389733CBF5DBD5E9C8A9BA7A39EFB766F5D81C698A659EA7CC7F00164DA146DA9985D098DBDEAEC8D2DCF9CF1F528DBCF6B57BC7E6449061A352F6E88A58FB86F5D81C698A659EA73AA81AA40904B5D9A18204E546F3947CF8BD4E506CFA3D886136E347CC761E074AD6D5ED66289B523666184CF4C3C14F6136E347CC761E07725E5C173C3A84C38A6EFC3CE370B017BA3038C0950A5D36B5C8C57E37DE458B330BD67F2E7D9AF16D1867E19FE14079C09775C1D3CA48CF17B107DEF921CE791DD303D21008E298D5E8D9A59859A8B6D082881546D9349175ECD9A6C639B01B78DA827A17800CE778B471BB9634AD8A731C566533BA786AA5CC5B56E945C8DA X-C1DE0DAB: 0D63561A33F958A5F3456E8F34A23F2C5002B1117B3ED696680E9DD20C03486C5B6221DB6D7A72AD823CB91A9FED034534781492E4B8EEADB73CFAAED92B6E13BDAD6C7F3747799A X-C8649E89: 1C3962B70DF3F0ADBF74143AD284FC7177DD89D51EBB7742424CF958EAFF5D571004E42C50DC4CA955A7F0CF078B5EC49A30900B95165D34D041FB2E16F174C4040074C0189679E618A50AD879BD5E63DF1D40637A87B54099B09024C1A4C8E11D7E09C32AA3244C54F0C82D89FE5C52EBBA027F22CEDE66FC41F5ECCBEB714AEA455F16B58544A2557BDE0DD54B3590A5AE236DF995FB59829709634694AABAED6A17656DB59BCAD427812AF56FC65B X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojbYjv4KtHp4HNzCUO5fNgdg== X-Mailru-Sender: 520A125C2F17F0B1A9638AD358559B59B94FA8F2C8791DF1C591814E25D11F9F988F912272586084B7CBEF92542CD7C88B0A2698F12F5C9EC77752E0C033A69E86920BD37369036789A8C6A0E60D2BB63A5DB60FBEB33A8A0DA7A0AF5A3A8387 X-Mras: Ok Subject: Re: [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 Kaplun via Tarantool-patches Reply-To: Sergey Kaplun Errors-To: tarantool-patches-bounces@dev.tarantool.org Sender: "Tarantool-patches" Hi, Sergey! Thanks for the fixes! On 23.07.24, Sergey Bronnikov wrote: > Hi, > > please see comments below. Fixes applied and force-pushed. > > Sergey > > On 10.07.2024 16:13, Sergey Kaplun wrote: > > Hi, Sergey! > > Thanks for the fixes! > > Please consider my minor nits about comments below. > > > > On 09.07.24, Sergey Bronnikov wrote: > >> Hi, Sergey, > >> > >> thanks for review. Fixes applied and force-pushed. > >> > >> Sergey > >> > >> > >> On 09.07.2024 14:52, Sergey Kaplun via Tarantool-patches wrote: > >>> Hi, Sergey! > >>> Thanks for the patch! > >>> Please consider my comments below. > >>> > >>> On 09.07.24, Sergey Bronnikov wrote: > >>>> From: Mike Pall > >>>> > > > > It is more correct to say, that "`lua_State` is marked after the > > function is removed from it" (since we stop the GC before chunk > > loading and starts after). My bad: Typo: s/starts/start/ > > > >>> Also, it is worth mentioning that the problem was partially solved, the > >>> complete fix will be applied in the next patch. > Added. > > Please, add its description to the commit message too. I would rephrase this part as the following: | 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 + > >>>> ...free-on-access-to-CTState-finalizer.test.c | 66 +++++++++++++++++++ > >>>> ...ee-on-access-to-CTState-finalizer.test.lua | 18 +++++ > >>>> 3 files changed, 87 insertions(+) > >>>> create mode 100644 test/tarantool-c-tests/lj-1168-heap-use-after-free-on-access-to-CTState-finalizer.test.c > >>>> create mode 100644 test/tarantool-tests/lj-1168-heap-use-after-free-on-access-to-CTState-finalizer.test.lua > > > > > >> > >>>> + * 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 was marked before the function is > > It is more correct to say, that "`lua_State` is marked after the > > function is removed from it" (since we stop the GC before chunk > > loading and starts after). My bad: Typo: s/starts/start/ > > > >>>> + if (luaL_loadbufferx(L, buff, sizeof(buff) - 1, "chunk", "t") != LUA_OK) > >>> Why do we need to omit the ending zero byte? Please add a comment that the terminating '\0' is considered by parser as part of the input, so we must chomp it. > > > > > > > >>>> diff --git a/test/tarantool-tests/lj-1168-heap-use-after-free-on-access-to-CTState-finalizer.test.lua b/test/tarantool-tests/lj-1168-heap-use-after-free-on-access-to-CTState-finalizer.test.lua > >>>> new file mode 100644 > >>>> index 00000000..fca5ec76 > >>>> --- /dev/null > >>>> +++ b/test/tarantool-tests/lj-1168-heap-use-after-free-on-access-to-CTState-finalizer.test.lua > >>>> 2.34.1 > >>>> -- Best regards, Sergey Kaplun