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 743615EE2D3; Sun, 3 Sep 2023 11:17:56 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 743615EE2D3 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=tarantool.org; s=dev; t=1693729076; bh=ojTg8opMGcIgi76W0040M573zX+nri2hKHZX6JDr6pM=; 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=jkqEcc2sIOqDtuMnR41gNPmOQszc3gMWxfeS5j/YP/dngaj/JVBDRgezqJAJYB1At tTVsfqENL1qqJ07SNfYR0EjGwktcNw2FzVgcNZTk1qw0n9KZ1qS1vJYGZTYQ080lY2 YGRMXZ+8N7z9Jg0MB5SgptwjlCw/ayFLx4pO0/Ks= 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 4F9994512A0 for ; Sun, 3 Sep 2023 11:17:55 +0300 (MSK) DKIM-Filter: OpenDKIM Filter v2.11.0 dev.tarantool.org 4F9994512A0 Received: by smtpng1.m.smailru.net with esmtpa (envelope-from ) id 1qciIs-0004Os-7f; Sun, 03 Sep 2023 11:17:54 +0300 Date: Sun, 3 Sep 2023 11:13:11 +0300 To: Igor Munkin Message-ID: References: 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: EEAE043A70213CC8 X-77F55803: 4F1203BC0FB41BD96201AD55A1C8F7DB4B94316895B7D143959E6F65F85AE25C182A05F5380850404C228DA9ACA6FE27589E7F7DA3927939DF747433939AA7BCE78E20136DA5CEAB86B46152C3E214E5 X-7FA49CB5: FF5795518A3D127A4AD6D5ED66289B5278DA827A17800CE7A548C5E803E75135EA1F7E6F0F101C67BD4B6F7A4D31EC0BCC500DACC3FED6E28638F802B75D45FF8AA50765F7900637D82ED515D6052E03EA1F7E6F0F101C6723150C8DA25C47586E58E00D9D99D84E1BDDB23E98D2D38BE5CCB53A13BC8DBA0E92003085DFC8DE5816EC0320E9748ECC7F00164DA146DAFE8445B8C89999728AA50765F79006373F278AD70EDB56F1389733CBF5DBD5E9C8A9BA7A39EFB766F5D81C698A659EA7CC7F00164DA146DA9985D098DBDEAEC817119E5299B287EEF6B57BC7E6449061A352F6E88A58FB86F5D81C698A659EA7E827F84554CEF5019E625A9149C048EE9ECD01F8117BC8BEE2021AF6380DFAD18AA50765F790063735872C767BF85DA227C277FBC8AE2E8B693784CDF876D4B675ECD9A6C639B01B4E70A05D1297E1BBCB5012B2E24CD356 X-C1DE0DAB: 0D63561A33F958A59191B267F9A851E50133732DDCF8586DF13DB659002BD924F87CCE6106E1FC07E67D4AC08A07B9B064E7220B7C550592CB5012B2E24CD356 X-C8649E89: 1C3962B70DF3F0ADE00A9FD3E00BEEDF3FED46C3ACD6F73ED3581295AF09D3DF87807E0823442EA2ED31085941D9CD0AF7F820E7B07EA4CF3CF76273027B9EB94B6AEABECD601AC9753484F5674A4447E1D489D99C599F144D6692FC0148BA75CB50FCC00789692A9CEB36E43E932F1DCBFE46D7BA1B172FE48CAC7CA610320002C26D483E81D6BE5EF9655DD6DEA7D65774BB76CC95456EEC5B5AD62611EEC62B5AFB4261A09AF0 X-D57D3AED: 3ZO7eAau8CL7WIMRKs4sN3D3tLDjz0dLbV79QFUyzQ2Ujvy7cMT6pYYqY16iZVKkSc3dCLJ7zSJH7+u4VD18S7Vl4ZUrpaVfd2+vE6kuoey4m4VkSEu530nj6fImhcD4MUrOEAnl0W826KZ9Q+tr5ycPtXkTV4k65bRjmOUUP8cvGozZ33TWg5HZplvhhXbhDGzqmQDTd6OAevLeAnq3Ra9uf7zvY2zzsIhlcp/Y7m53TZgf2aB4JOg4gkr2biojBdR/qdXWEGMKyDQiMSgB2w== X-DA7885C5: DF139136AB1881A5FC8EA273F796804A1259BF6A5079881A770BF4568A543960262E2D401490A4A0DB037EFA58388B346E8BC1A9835FDE71 X-Mailru-Sender: 689FA8AB762F73930F533AC2B33E986BE86D605FC767783DE6196E986A4E363B0FBE9A32752B8C9C2AA642CC12EC09F1FB559BB5D741EB962F61BD320559CF1EFD657A8799238ED55FEEDEB644C299C0ED14614B50AE0675 X-Mras: Ok Subject: Re: [Tarantool-patches] [PATCH luajit] Always exit after machine code page protection change fails. 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, Igor! Thanks for the patch! Please, consider my comments below. On 01.09.23, Igor Munkin wrote: > From: Mike Pall > > Reported by Sergey Kaplun. > > (cherry picked from commit c50232eb320d56d526ba5e6cb5bda8cf5a848a55) > > Unfortunately, call had been missing for a long time for the case > when fails within . Though the patch per se is > quite trivial, the test is not at all. It exploits the fact, that Typo: s/fact,/fact/ > is used only for protecting area for mcode or callback Typo: s/area/the area/ > function pointers. Hence, if the test doesn't use FFI at all, it is > guaranteed that the only called in LuaJIT runtime locates in > (that is not true for Tarantool, so the test is disabled > for integration testing routine). Furthermore, overloading on Typo: s/for/for the/ > macOS occurs to be not an easy ride either, so running the test on macOS s/occurs to be/is/ Feel free to ignore. > is disabled, since this is the common part for all platforms and Typo: s/disabled,/disabled/ > everything can be checked on Linux in a much more easier way. Typo: s/more// > > Igor Munkin: > * added the description and the test for the problem > > Part of tarantool/tarantool#8825 > > Signed-off-by: Igor Munkin > --- > > Branch: https://github.com/tarantool/luajit/tree/imun/lj-802-panic-at-mcode-protfail > Tarantool PR: https://github.com/tarantool/tarantool/pull/9077 > Related issues: > * https://github.com/tarantool/tarantool/issues/8825 > * https://github.com/LuaJIT/LuaJIT/issues/802 > > Regarding the macOS and overriding: I tried several flags to > make this crap work, but I've finally failed to override via > the alternative dylib. I add the related changes right below, so anyone > curious can try my approach. > > ================================================================================ > > diff --git a/test/tarantool-tests/lj-802-panic-at-mcode-protfail.test.lua b/test/tarantool-tests/lj-802-panic-at-mcode-protfail.test.lua > index 83a9ae2e..3f28e420 100644 > --- a/test/tarantool-tests/lj-802-panic-at-mcode-protfail.test.lua > +++ b/test/tarantool-tests/lj-802-panic-at-mcode-protfail.test.lua > @@ -24,7 +24,11 @@ test:plan(3) > -- runs %testname%/script.lua by > -- with the given environment, launch options and CLI arguments. > local script = require('utils').exec.makecmd(arg, { > - env = { LD_PRELOAD = 'mymprotect.so' }, > + env = { > + DYLD_LIBRARY_PATH = os.getenv('DYLD_LIBRARY_PATH'), > + DYLD_INSERT_LIBRARIES = 'mymprotect.dylib', > + LD_PRELOAD = 'mymprotect.so', > + }, > redirect = '2>&1', > }) > > ================================================================================ > > This link[1] claims, that there is no way to override syscalls this way > on macOS, but I've even failed to override or rather . > DYLD_FORCE_FLATE_NAMESPACE didn't change the result, all DYLD_* > debugging envvars yield some info, that looks like a gibberish to me > (comparing to LD_DEBUG=1). I'm giving up (at least for now), but I'm > open to discuss if someone is interested in debugging this. > > [1]: https://stackoverflow.com/questions/929893/how-can-i-override-malloc-calloc-free-etc-under-os-x Thanks for the clarification! I suppose that skipcond for macOS is fine. > > src/lj_mcode.c | 3 +- > test/tarantool-tests/CMakeLists.txt | 1 + > .../lj-802-panic-at-mcode-protfail.test.lua | 41 +++++++++++++++++++ > .../CMakeLists.txt | 1 + > .../mymprotect.c | 6 +++ > .../lj-802-panic-at-mcode-protfail/script.lua | 12 ++++++ > 6 files changed, 63 insertions(+), 1 deletion(-) > create mode 100644 test/tarantool-tests/lj-802-panic-at-mcode-protfail.test.lua > create mode 100644 test/tarantool-tests/lj-802-panic-at-mcode-protfail/CMakeLists.txt > create mode 100644 test/tarantool-tests/lj-802-panic-at-mcode-protfail/mymprotect.c > create mode 100644 test/tarantool-tests/lj-802-panic-at-mcode-protfail/script.lua > > diff --git a/test/tarantool-tests/lj-802-panic-at-mcode-protfail.test.lua b/test/tarantool-tests/lj-802-panic-at-mcode-protfail.test.lua > new file mode 100644 > index 00000000..83a9ae2e > --- /dev/null > +++ b/test/tarantool-tests/lj-802-panic-at-mcode-protfail.test.lua The test doesn't fail before the patch Neither if run it from command line: | LUA_PATH="src/?.lua;test/tarantool-tests/?.lua;test/tarantool-tests/?/init.lua;" LD_LIBRARY_PATH="test/tarantool-tests/lj-802-panic-at-mcode-protfail/" src/luajit test/tarantool-tests/lj-802-panic-at-mcode-protfail.test.lua | TAP version 13 | 1..3 | ok - Panic occurred as a result of failure | ok - LuaJIT exited as a result of the panic (error check) | ok - LuaJIT exited as a result of the panic (poison check) nor as as part of `make tarantool-tests` (checked with or wo GC64). Compiled as the following: | cmake . -DLUAJIT_ENABLE_WARNINGS=OFF -DCMAKE_BUILD_TYPE=Debug -DLUA_USE_APICHECK=ON -DLUA_USE_ASSERT=ON -DLUAJIT_ENABLE_GC64=ON && make -j Tested with the following diff: =================================================================== diff --git a/src/lj_mcode.c b/src/lj_mcode.c index a88d16bd..9b59053a 100644 --- a/src/lj_mcode.c +++ b/src/lj_mcode.c @@ -180,7 +180,7 @@ static void mcode_protect(jit_State *J, int prot) #define MCPROT_RUN MCPROT_RX /* Protection twiddling failed. Probably due to kernel security. */ -static LJ_NORET LJ_NOINLINE void mcode_protfail(jit_State *J) +static LJ_NOINLINE void mcode_protfail(jit_State *J) { lua_CFunction panic = J2G(J)->panic; if (panic) { @@ -188,7 +188,7 @@ static LJ_NORET LJ_NOINLINE void mcode_protfail(jit_State *J) setstrV(L, L->top++, lj_err_str(L, LJ_ERR_JITPROT)); panic(L); } - exit(EXIT_FAILURE); + // exit(EXIT_FAILURE); } /* Change protection of MCode area. */ =================================================================== WDIDW? > @@ -0,0 +1,41 @@ > +local tap = require('tap') > +local test = tap.test('lj-flush-on-trace'):skipcond({ > + ['Test requires JIT enabled'] = not jit.status(), > + ['Disabled on *BSD due to #4819'] = jit.os == 'BSD', > + -- XXX: This test has to check the particular patch for > + -- and is overloaded for this Typo: s//,/ > + -- purpose. However, is used widely in Tarantool Typo: s/is used widely/is widely used/ > + -- to play with fiber stacks, so overriding is not > + -- suitable to test this feature in Tarantool. > + -- luacheck: no global > + [' overriding can break Tarantool'] = _TARANTOOL, > + -- XXX: Unfortunately, it's too hard to overload (or even > + -- impossible, who knows, since Cupertino fellows do not > + -- provide any information about their system) something from > + -- libsystem_kernel.dylib (the library providing ). > + -- All in all, this test checks the part, that is common for all Typo: s/part,/part/ > + -- platforms, so it's not vital to run this test on macOS, since Typo: s/macOS,/macOS/ > + -- everything can be checked on Linux in a much more easier way. Typo: s/more// > + [' cannot be overridden on macOS'] = jit.os == 'OSX', > +}) > + > +test:plan(3) > + > diff --git a/test/tarantool-tests/lj-802-panic-at-mcode-protfail/script.lua b/test/tarantool-tests/lj-802-panic-at-mcode-protfail/script.lua > new file mode 100644 > index 00000000..661099fa > --- /dev/null > +++ b/test/tarantool-tests/lj-802-panic-at-mcode-protfail/script.lua > @@ -0,0 +1,12 @@ > +jit.opt.start('hotloop=1') > + > +-- Run a simple loop that triggers on trace assembling. > +local a = 0 > +for i = 1, 3 do Minor: Should it be 4 here to actually *run* a loop and be sure that we don't execute this part of the code? > + a = a + i > +end > + > +-- XXX: Just a simple contract output in case neither panic at Typo: s/panic/the panic/ > +-- , nor crash occurs (see for LUAJIT_UNPROTECT_MCODE in > +-- lj_mcode.c for more info). > +io.write(arg[1]) > -- > 2.30.2 > -- Best regards, Sergey Kaplun