Tarantool development patches archive
 help / color / mirror / Atom feed
* [Tarantool-patches] [PATCH luajit] Always exit after machine code page protection change fails.
@ 2023-09-01 12:46 Igor Munkin via Tarantool-patches
  2023-09-03  8:13 ` Sergey Kaplun via Tarantool-patches
  2023-09-27 12:33 ` Igor Munkin via Tarantool-patches
  0 siblings, 2 replies; 6+ messages in thread
From: Igor Munkin via Tarantool-patches @ 2023-09-01 12:46 UTC (permalink / raw)
  To: Maxim Kokryashkin, Sergey Kaplun; +Cc: tarantool-patches

From: Mike Pall <mike>

Reported by Sergey Kaplun.

(cherry picked from commit c50232eb320d56d526ba5e6cb5bda8cf5a848a55)

Unfortunately, <exit> call had been missing for a long time for the case
when <mprotect> fails within <mcode_protect>. Though the patch per se is
quite trivial, the test is not at all. It exploits the fact, that
<mprotect> is used only for protecting area for mcode or callback
function pointers. Hence, if the test doesn't use FFI at all, it is
guaranteed that the only <mprotect> called in LuaJIT runtime locates in
<mcode_protect> (that is not true for Tarantool, so the test is disabled
for integration testing routine). Furthermore, overloading <mprotect> on
macOS occurs to be not an easy ride either, so running the test on macOS
is disabled, since this is the common part for all platforms and
everything can be checked on Linux in a much more easier way.

Igor Munkin:
* added the description and the test for the problem

Part of tarantool/tarantool#8825

Signed-off-by: Igor Munkin <imun@tarantool.org>
---

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 <mprotect> overriding: I tried several flags to
make this crap work, but I've finally failed to override <mprotect> 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)
 -- <makecmd> runs %testname%/script.lua by <LUAJIT_TEST_BINARY>
 -- 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 <malloc> or rather <memcpy>.
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

 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/src/lj_mcode.c b/src/lj_mcode.c
index 808a9897..a88d16bd 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_NOINLINE void mcode_protfail(jit_State *J)
+static LJ_NORET LJ_NOINLINE void mcode_protfail(jit_State *J)
 {
   lua_CFunction panic = J2G(J)->panic;
   if (panic) {
@@ -188,6 +188,7 @@ static LJ_NOINLINE void mcode_protfail(jit_State *J)
     setstrV(L, L->top++, lj_err_str(L, LJ_ERR_JITPROT));
     panic(L);
   }
+  exit(EXIT_FAILURE);
 }
 
 /* Change protection of MCode area. */
diff --git a/test/tarantool-tests/CMakeLists.txt b/test/tarantool-tests/CMakeLists.txt
index 6218f76a..64c0319b 100644
--- a/test/tarantool-tests/CMakeLists.txt
+++ b/test/tarantool-tests/CMakeLists.txt
@@ -65,6 +65,7 @@ add_subdirectory(gh-6189-cur_L)
 add_subdirectory(lj-416-xor-before-jcc)
 add_subdirectory(lj-601-fix-gc-finderrfunc)
 add_subdirectory(lj-727-lightuserdata-itern)
+add_subdirectory(lj-802-panic-at-mcode-protfail)
 add_subdirectory(lj-flush-on-trace)
 
 # The part of the memory profiler toolchain is located in tools
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
@@ -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
+  -- <mcode_protfail> and <mprotect> is overloaded for this
+  -- purpose. However, <mprotect> is used widely in Tarantool
+  -- to play with fiber stacks, so overriding <mprotect> is not
+  -- suitable to test this feature in Tarantool.
+  -- luacheck: no global
+  ['<mprotect> 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 <mprotect>).
+  -- All in all, this test checks the part, that is common for all
+  -- platforms, so it's not vital to run this test on macOS, since
+  -- everything can be checked on Linux in a much more easier way.
+  ['<mprotect> cannot be overridden on macOS'] = jit.os == 'OSX',
+})
+
+test:plan(3)
+
+-- <makecmd> runs %testname%/script.lua by <LUAJIT_TEST_BINARY>
+-- with the given environment, launch options and CLI arguments.
+local script = require('utils').exec.makecmd(arg, {
+  env = { LD_PRELOAD = 'mymprotect.so' },
+  redirect = '2>&1',
+})
+
+-- See the rationale for this poison hack in the script.lua.
+local poison = '"runtime code generation succeed"'
+local output = script(poison)
+test:like(output, 'runtime code generation failed, restricted kernel%?',
+          'Panic occurred as a result of <mprotect> failure')
+test:unlike(output, 'Segmentation fault',
+            'LuaJIT exited as a result of the panic (error check)')
+test:unlike(output, poison,
+            'LuaJIT exited as a result of the panic (poison check)')
+
+test:done(true)
diff --git a/test/tarantool-tests/lj-802-panic-at-mcode-protfail/CMakeLists.txt b/test/tarantool-tests/lj-802-panic-at-mcode-protfail/CMakeLists.txt
new file mode 100644
index 00000000..25520a1a
--- /dev/null
+++ b/test/tarantool-tests/lj-802-panic-at-mcode-protfail/CMakeLists.txt
@@ -0,0 +1 @@
+BuildTestCLib(mymprotect mymprotect.c)
diff --git a/test/tarantool-tests/lj-802-panic-at-mcode-protfail/mymprotect.c b/test/tarantool-tests/lj-802-panic-at-mcode-protfail/mymprotect.c
new file mode 100644
index 00000000..65763b1b
--- /dev/null
+++ b/test/tarantool-tests/lj-802-panic-at-mcode-protfail/mymprotect.c
@@ -0,0 +1,6 @@
+#include <stddef.h>
+
+int mprotect(void *addr, size_t len, int prot)
+{
+	return -1;
+}
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 <mprotect> on trace assembling.
+local a = 0
+for i = 1, 3 do
+  a = a + i
+end
+
+-- XXX: Just a simple contract output in case neither panic at
+-- <mprotect>, nor crash occurs (see for LUAJIT_UNPROTECT_MCODE in
+-- lj_mcode.c for more info).
+io.write(arg[1])
-- 
2.30.2


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Tarantool-patches] [PATCH luajit] Always exit after machine code page protection change fails.
  2023-09-01 12:46 [Tarantool-patches] [PATCH luajit] Always exit after machine code page protection change fails Igor Munkin via Tarantool-patches
@ 2023-09-03  8:13 ` Sergey Kaplun via Tarantool-patches
  2023-09-05 11:31   ` Igor Munkin via Tarantool-patches
  2023-09-27 12:33 ` Igor Munkin via Tarantool-patches
  1 sibling, 1 reply; 6+ messages in thread
From: Sergey Kaplun via Tarantool-patches @ 2023-09-03  8:13 UTC (permalink / raw)
  To: Igor Munkin; +Cc: tarantool-patches

Hi, Igor!
Thanks for the patch!
Please, consider my comments below.

On 01.09.23, Igor Munkin wrote:
> From: Mike Pall <mike>
> 
> Reported by Sergey Kaplun.
> 
> (cherry picked from commit c50232eb320d56d526ba5e6cb5bda8cf5a848a55)
> 
> Unfortunately, <exit> call had been missing for a long time for the case
> when <mprotect> fails within <mcode_protect>. Though the patch per se is
> quite trivial, the test is not at all. It exploits the fact, that

Typo: s/fact,/fact/

> <mprotect> 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 <mprotect> called in LuaJIT runtime locates in
> <mcode_protect> (that is not true for Tarantool, so the test is disabled
> for integration testing routine). Furthermore, overloading <mprotect> 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 <imun@tarantool.org>
> ---
> 
> 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 <mprotect> overriding: I tried several flags to
> make this crap work, but I've finally failed to override <mprotect> 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)
>  -- <makecmd> runs %testname%/script.lua by <LUAJIT_TEST_BINARY>
>  -- 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 <malloc> or rather <memcpy>.
> 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
> 

<snipped>

> 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 <mprotect> 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
> +  -- <mcode_protfail> and <mprotect> is overloaded for this

Typo: s/<mcode_protfail>/<mcode_protfail>,/

> +  -- purpose. However, <mprotect> is used widely in Tarantool

Typo: s/is used widely/is widely used/

> +  -- to play with fiber stacks, so overriding <mprotect> is not
> +  -- suitable to test this feature in Tarantool.
> +  -- luacheck: no global
> +  ['<mprotect> 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 <mprotect>).
> +  -- 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//

> +  ['<mprotect> cannot be overridden on macOS'] = jit.os == 'OSX',
> +})
> +
> +test:plan(3)
> +

<snipped>

> 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 <mprotect> 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/

> +-- <mprotect>, 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

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Tarantool-patches] [PATCH luajit] Always exit after machine code page protection change fails.
  2023-09-03  8:13 ` Sergey Kaplun via Tarantool-patches
@ 2023-09-05 11:31   ` Igor Munkin via Tarantool-patches
  2023-09-05 14:20     ` Sergey Kaplun via Tarantool-patches
  2023-09-07 10:31     ` Maxim Kokryashkin via Tarantool-patches
  0 siblings, 2 replies; 6+ messages in thread
From: Igor Munkin via Tarantool-patches @ 2023-09-05 11:31 UTC (permalink / raw)
  To: Sergey Kaplun; +Cc: tarantool-patches

Sergey,

Thanks for your review! I've replied to your comments inline.

On 03.09.23, Sergey Kaplun wrote:
> Hi, Igor!
> Thanks for the patch!
> Please, consider my comments below.
> 
> On 01.09.23, Igor Munkin wrote:
> > From: Mike Pall <mike>
> > 
> > Reported by Sergey Kaplun.
> > 
> > (cherry picked from commit c50232eb320d56d526ba5e6cb5bda8cf5a848a55)
> > 
> > Unfortunately, <exit> call had been missing for a long time for the case
> > when <mprotect> fails within <mcode_protect>. Though the patch per se is
> > quite trivial, the test is not at all. It exploits the fact, that
> 
> Typo: s/fact,/fact/
> 
> > <mprotect> 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 <mprotect> called in LuaJIT runtime locates in
> > <mcode_protect> (that is not true for Tarantool, so the test is disabled
> > for integration testing routine). Furthermore, overloading <mprotect> 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//
> 
> > 

Thanks for proofreading the commit message; the new one with the fixes
can be found below:

================================================================================

Always exit after machine code page protection change fails.

Reported by Sergey Kaplun.

(cherry picked from commit c50232eb320d56d526ba5e6cb5bda8cf5a848a55)

Unfortunately, <exit> call had been missing for a long time for the case
when <mprotect> fails within <mcode_protect>. Though the patch per se is
quite trivial, the test is not at all. It exploits the fact that
<mprotect> is used only for protecting the area for mcode or callback
function pointers. Hence, if the test doesn't use FFI at all, it is
guaranteed that the only <mprotect> called in LuaJIT runtime locates in
<mcode_protect> (that is not true for Tarantool, so the test is disabled
for the integration testing routine). Furthermore, attempts to overload
<mprotect> on macOS occur to be not an easy ride either, so running the
test on macOS is disabled since this is the common part for all
platforms and everything can be checked on Linux in a much easier way.

Igor Munkin:
* added the description and the test for the problem

Part of tarantool/tarantool#8825

================================================================================

> > Igor Munkin:
> > * added the description and the test for the problem
> > 
> > Part of tarantool/tarantool#8825
> > 
> > Signed-off-by: Igor Munkin <imun@tarantool.org>
> > ---
> > 
> > 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
> > 

<snipped>

> > 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 <mprotect> 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?

Building LuaJIT with the internal assertions, LOL. Anyway, thanks for
noticing this. I've added additional check to the test (see the
incremental diff below):

================================================================================

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..f4dc4e1c 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
@@ -19,7 +19,7 @@ local test = tap.test('lj-flush-on-trace'):skipcond({
   ['<mprotect> cannot be overridden on macOS'] = jit.os == 'OSX',
 })
 
-test:plan(3)
+test:plan(4)
 
 -- <makecmd> runs %testname%/script.lua by <LUAJIT_TEST_BINARY>
 -- with the given environment, launch options and CLI arguments.
@@ -35,6 +35,8 @@ test:like(output, 'runtime code generation failed, restricted kernel%?',
           'Panic occurred as a result of <mprotect> failure')
 test:unlike(output, 'Segmentation fault',
             'LuaJIT exited as a result of the panic (error check)')
+test:unlike(output, 'Aborted',
+            'LuaJIT exited as a result of the panic (assertion check)')
 test:unlike(output, poison,
             'LuaJIT exited as a result of the panic (poison check)')
 

================================================================================

> 
> > @@ -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
> > +  -- <mcode_protfail> and <mprotect> is overloaded for this
> 
> Typo: s/<mcode_protfail>/<mcode_protfail>,/
> 
> > +  -- purpose. However, <mprotect> is used widely in Tarantool
> 
> Typo: s/is used widely/is widely used/
> 
> > +  -- to play with fiber stacks, so overriding <mprotect> is not
> > +  -- suitable to test this feature in Tarantool.
> > +  -- luacheck: no global
> > +  ['<mprotect> 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 <mprotect>).
> > +  -- 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//
> 
> > +  ['<mprotect> cannot be overridden on macOS'] = jit.os == 'OSX',
> > +})
> > +
> > +test:plan(3)
> > +

All the typos in the test code are fixed; the diff is below:

================================================================================

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 f4dc4e1c..94f4314f 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
@@ -3,8 +3,8 @@ 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
-  -- <mcode_protfail> and <mprotect> is overloaded for this
-  -- purpose. However, <mprotect> is used widely in Tarantool
+  -- <mcode_protfail>, and <mprotect> is overloaded for this
+  -- purpose. However, <mprotect> is widely used in Tarantool
   -- to play with fiber stacks, so overriding <mprotect> is not
   -- suitable to test this feature in Tarantool.
   -- luacheck: no global
@@ -13,9 +13,9 @@ local test = tap.test('lj-flush-on-trace'):skipcond({
   -- impossible, who knows, since Cupertino fellows do not
   -- provide any information about their system) something from
   -- libsystem_kernel.dylib (the library providing <mprotect>).
-  -- All in all, this test checks the part, that is common for all
-  -- platforms, so it's not vital to run this test on macOS, since
-  -- everything can be checked on Linux in a much more easier way.
+  -- All in all, this test checks the part that is common for all
+  -- platforms, so it's not vital to run this test on macOS since
+  -- everything can be checked on Linux in a much easier way.
   ['<mprotect> cannot be overridden on macOS'] = jit.os == 'OSX',
 })
 

================================================================================

> > 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 <mprotect> 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?

But 3 is this dedicated iteration. Here are some details regarding the
loop recording:
* The first iteration is executed until the corresponding loop bytecode
  (BC_FORL, IIRC) is reached.
* Starting from the first occurrence of the aforementioned bytecode, the
  compiler starts recording the loop. This is the second iteration.
* To successfully finalize the compilation of the loop, the jump at that
  bytecode should be done back to the beginning of the loop body (this
  is the heuristic of JIT compiler, since the loop is not considered
  "hot" otherwise and there is no reason to finalize this trace). If the
  jump target is valid in the sense described above, the trace is
  finalized and, since the jump targets back to the loop body, the next
  loop iteration is run.
* Since the trace is already compiled, the third and the last iteration
  is run via the mcode instead of VM interpreting the bytecode. At this
  point the execution of the trace is finished with the corresponding
  guard checking the "iterator" value.

So, if you want one "full" iteration and one "to-be-exited" iteration, I
can increment the right boundary of the loop, but I doubt that it's
strongly required here (since we check that the compilation is failed at
the assembling phase).

> 
> > +  a = a + i
> > +end
> > +
> > +-- XXX: Just a simple contract output in case neither panic at
> 
> Typo: s/panic/the panic/
> 

All the typos in the script code are also fixed; the diff is below:

================================================================================

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
index 661099fa..201a8ff2 100644
--- a/test/tarantool-tests/lj-802-panic-at-mcode-protfail/script.lua
+++ b/test/tarantool-tests/lj-802-panic-at-mcode-protfail/script.lua
@@ -6,7 +6,7 @@ for i = 1, 3 do
   a = a + i
 end
 
--- XXX: Just a simple contract output in case neither panic at
+-- XXX: Just a simple contract output in case neither the panic at
 -- <mprotect>, nor crash occurs (see for LUAJIT_UNPROTECT_MCODE in
 -- lj_mcode.c for more info).
 io.write(arg[1])

================================================================================

> > +-- <mprotect>, 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

-- 
Best regards,
IM

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Tarantool-patches] [PATCH luajit] Always exit after machine code page protection change fails.
  2023-09-05 11:31   ` Igor Munkin via Tarantool-patches
@ 2023-09-05 14:20     ` Sergey Kaplun via Tarantool-patches
  2023-09-07 10:31     ` Maxim Kokryashkin via Tarantool-patches
  1 sibling, 0 replies; 6+ messages in thread
From: Sergey Kaplun via Tarantool-patches @ 2023-09-05 14:20 UTC (permalink / raw)
  To: Igor Munkin; +Cc: tarantool-patches

Hi, Igor!
Thanks for the fixes!
LGTM!

-- 
Best regards,
Sergey Kaplun

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Tarantool-patches]  [PATCH luajit] Always exit after machine code page protection change fails.
  2023-09-05 11:31   ` Igor Munkin via Tarantool-patches
  2023-09-05 14:20     ` Sergey Kaplun via Tarantool-patches
@ 2023-09-07 10:31     ` Maxim Kokryashkin via Tarantool-patches
  1 sibling, 0 replies; 6+ messages in thread
From: Maxim Kokryashkin via Tarantool-patches @ 2023-09-07 10:31 UTC (permalink / raw)
  To: Igor Munkin; +Cc: tarantool-patches

[-- Attachment #1: Type: text/plain, Size: 13460 bytes --]


Hi, Igor!
Thanks for the patch and the fixes!
LGTM
 
--
Best regards,
Maxim Kokryashkin
 
  
>Вторник, 5 сентября 2023, 14:49 +03:00 от Igor Munkin <imun@tarantool.org>:
> 
>Sergey,
>
>Thanks for your review! I've replied to your comments inline.
>
>On 03.09.23, Sergey Kaplun wrote:
>> Hi, Igor!
>> Thanks for the patch!
>> Please, consider my comments below.
>>
>> On 01.09.23, Igor Munkin wrote:
>> > From: Mike Pall <mike>
>> >
>> > Reported by Sergey Kaplun.
>> >
>> > (cherry picked from commit c50232eb320d56d526ba5e6cb5bda8cf5a848a55)
>> >
>> > Unfortunately, <exit> call had been missing for a long time for the case
>> > when <mprotect> fails within <mcode_protect>. Though the patch per se is
>> > quite trivial, the test is not at all. It exploits the fact, that
>>
>> Typo: s/fact,/fact/
>>
>> > <mprotect> 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 <mprotect> called in LuaJIT runtime locates in
>> > <mcode_protect> (that is not true for Tarantool, so the test is disabled
>> > for integration testing routine). Furthermore, overloading <mprotect> 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//
>>
>> >
>
>Thanks for proofreading the commit message; the new one with the fixes
>can be found below:
>
>================================================================================
>
>Always exit after machine code page protection change fails.
>
>Reported by Sergey Kaplun.
>
>(cherry picked from commit c50232eb320d56d526ba5e6cb5bda8cf5a848a55)
>
>Unfortunately, <exit> call had been missing for a long time for the case
>when <mprotect> fails within <mcode_protect>. Though the patch per se is
>quite trivial, the test is not at all. It exploits the fact that
><mprotect> is used only for protecting the area for mcode or callback
>function pointers. Hence, if the test doesn't use FFI at all, it is
>guaranteed that the only <mprotect> called in LuaJIT runtime locates in
><mcode_protect> (that is not true for Tarantool, so the test is disabled
>for the integration testing routine). Furthermore, attempts to overload
><mprotect> on macOS occur to be not an easy ride either, so running the
>test on macOS is disabled since this is the common part for all
>platforms and everything can be checked on Linux in a much easier way.
>
>Igor Munkin:
>* added the description and the test for the problem
>
>Part of tarantool/tarantool#8825
>
>================================================================================
>
>> > Igor Munkin:
>> > * added the description and the test for the problem
>> >
>> > Part of tarantool/tarantool#8825
>> >
>> > Signed-off-by: Igor Munkin < imun@tarantool.org >
>> > ---
>> >
>> > 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
>> >
>
><snipped>
>
>> > 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 <mprotect> 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?
>
>Building LuaJIT with the internal assertions, LOL. Anyway, thanks for
>noticing this. I've added additional check to the test (see the
>incremental diff below):
>
>================================================================================
>
>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..f4dc4e1c 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
>@@ -19,7 +19,7 @@ local test = tap.test('lj-flush-on-trace'):skipcond({
>   ['<mprotect> cannot be overridden on macOS'] = jit.os == 'OSX',
> })
> 
>-test:plan(3)
>+test:plan(4)
> 
> -- <makecmd> runs %testname%/script.lua by <LUAJIT_TEST_BINARY>
> -- with the given environment, launch options and CLI arguments.
>@@ -35,6 +35,8 @@ test:like(output, 'runtime code generation failed, restricted kernel%?',
>           'Panic occurred as a result of <mprotect> failure')
> test:unlike(output, 'Segmentation fault',
>             'LuaJIT exited as a result of the panic (error check)')
>+test:unlike(output, 'Aborted',
>+ 'LuaJIT exited as a result of the panic (assertion check)')
> test:unlike(output, poison,
>             'LuaJIT exited as a result of the panic (poison check)')
> 
>
>================================================================================
>
>>
>> > @@ -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
>> > + -- <mcode_protfail> and <mprotect> is overloaded for this
>>
>> Typo: s/<mcode_protfail>/<mcode_protfail>,/
>>
>> > + -- purpose. However, <mprotect> is used widely in Tarantool
>>
>> Typo: s/is used widely/is widely used/
>>
>> > + -- to play with fiber stacks, so overriding <mprotect> is not
>> > + -- suitable to test this feature in Tarantool.
>> > + -- luacheck: no global
>> > + ['<mprotect> 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 <mprotect>).
>> > + -- 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//
>>
>> > + ['<mprotect> cannot be overridden on macOS'] = jit.os == 'OSX',
>> > +})
>> > +
>> > +test:plan(3)
>> > +
>
>All the typos in the test code are fixed; the diff is below:
>
>================================================================================
>
>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 f4dc4e1c..94f4314f 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
>@@ -3,8 +3,8 @@ 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
>- -- <mcode_protfail> and <mprotect> is overloaded for this
>- -- purpose. However, <mprotect> is used widely in Tarantool
>+ -- <mcode_protfail>, and <mprotect> is overloaded for this
>+ -- purpose. However, <mprotect> is widely used in Tarantool
>   -- to play with fiber stacks, so overriding <mprotect> is not
>   -- suitable to test this feature in Tarantool.
>   -- luacheck: no global
>@@ -13,9 +13,9 @@ local test = tap.test('lj-flush-on-trace'):skipcond({
>   -- impossible, who knows, since Cupertino fellows do not
>   -- provide any information about their system) something from
>   -- libsystem_kernel.dylib (the library providing <mprotect>).
>- -- All in all, this test checks the part, that is common for all
>- -- platforms, so it's not vital to run this test on macOS, since
>- -- everything can be checked on Linux in a much more easier way.
>+ -- All in all, this test checks the part that is common for all
>+ -- platforms, so it's not vital to run this test on macOS since
>+ -- everything can be checked on Linux in a much easier way.
>   ['<mprotect> cannot be overridden on macOS'] = jit.os == 'OSX',
> })
> 
>
>================================================================================
>
>> > 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 <mprotect> 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?
>
>But 3 is this dedicated iteration. Here are some details regarding the
>loop recording:
>* The first iteration is executed until the corresponding loop bytecode
>  (BC_FORL, IIRC) is reached.
>* Starting from the first occurrence of the aforementioned bytecode, the
>  compiler starts recording the loop. This is the second iteration.
>* To successfully finalize the compilation of the loop, the jump at that
>  bytecode should be done back to the beginning of the loop body (this
>  is the heuristic of JIT compiler, since the loop is not considered
>  "hot" otherwise and there is no reason to finalize this trace). If the
>  jump target is valid in the sense described above, the trace is
>  finalized and, since the jump targets back to the loop body, the next
>  loop iteration is run.
>* Since the trace is already compiled, the third and the last iteration
>  is run via the mcode instead of VM interpreting the bytecode. At this
>  point the execution of the trace is finished with the corresponding
>  guard checking the "iterator" value.
>
>So, if you want one "full" iteration and one "to-be-exited" iteration, I
>can increment the right boundary of the loop, but I doubt that it's
>strongly required here (since we check that the compilation is failed at
>the assembling phase).
>
>>
>> > + a = a + i
>> > +end
>> > +
>> > +-- XXX: Just a simple contract output in case neither panic at
>>
>> Typo: s/panic/the panic/
>>
>
>All the typos in the script code are also fixed; the diff is below:
>
>================================================================================
>
>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
>index 661099fa..201a8ff2 100644
>--- a/test/tarantool-tests/lj-802-panic-at-mcode-protfail/script.lua
>+++ b/test/tarantool-tests/lj-802-panic-at-mcode-protfail/script.lua
>@@ -6,7 +6,7 @@ for i = 1, 3 do
>   a = a + i
> end
> 
>--- XXX: Just a simple contract output in case neither panic at
>+-- XXX: Just a simple contract output in case neither the panic at
> -- <mprotect>, nor crash occurs (see for LUAJIT_UNPROTECT_MCODE in
> -- lj_mcode.c for more info).
> io.write(arg[1])
>
>================================================================================
>
>> > +-- <mprotect>, 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
>
>--
>Best regards,
>IM
 

[-- Attachment #2: Type: text/html, Size: 16442 bytes --]

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [Tarantool-patches] [PATCH luajit] Always exit after machine code page protection change fails.
  2023-09-01 12:46 [Tarantool-patches] [PATCH luajit] Always exit after machine code page protection change fails Igor Munkin via Tarantool-patches
  2023-09-03  8:13 ` Sergey Kaplun via Tarantool-patches
@ 2023-09-27 12:33 ` Igor Munkin via Tarantool-patches
  1 sibling, 0 replies; 6+ messages in thread
From: Igor Munkin via Tarantool-patches @ 2023-09-27 12:33 UTC (permalink / raw)
  To: Maxim Kokryashkin, Sergey Kaplun; +Cc: tarantool-patches

Pals, thanks for your reviews!

I've checked the patchset into all long-term branches in
tarantool/luajit and bumped a new version in master, release/2.11 and
release/2.10.

On 01.09.23, Igor Munkin wrote:
> From: Mike Pall <mike>
> 
> Reported by Sergey Kaplun.
> 
> (cherry picked from commit c50232eb320d56d526ba5e6cb5bda8cf5a848a55)
> 
> Unfortunately, <exit> call had been missing for a long time for the case
> when <mprotect> fails within <mcode_protect>. Though the patch per se is
> quite trivial, the test is not at all. It exploits the fact, that
> <mprotect> is used only for protecting area for mcode or callback
> function pointers. Hence, if the test doesn't use FFI at all, it is
> guaranteed that the only <mprotect> called in LuaJIT runtime locates in
> <mcode_protect> (that is not true for Tarantool, so the test is disabled
> for integration testing routine). Furthermore, overloading <mprotect> on
> macOS occurs to be not an easy ride either, so running the test on macOS
> is disabled, since this is the common part for all platforms and
> everything can be checked on Linux in a much more easier way.
> 
> Igor Munkin:
> * added the description and the test for the problem
> 
> Part of tarantool/tarantool#8825
> 
> Signed-off-by: Igor Munkin <imun@tarantool.org>
> ---
> 
> 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
> 

<snipped>

> -- 
> 2.30.2
> 

-- 
Best regards,
IM

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2023-09-27 12:54 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-09-01 12:46 [Tarantool-patches] [PATCH luajit] Always exit after machine code page protection change fails Igor Munkin via Tarantool-patches
2023-09-03  8:13 ` Sergey Kaplun via Tarantool-patches
2023-09-05 11:31   ` Igor Munkin via Tarantool-patches
2023-09-05 14:20     ` Sergey Kaplun via Tarantool-patches
2023-09-07 10:31     ` Maxim Kokryashkin via Tarantool-patches
2023-09-27 12:33 ` Igor Munkin via Tarantool-patches

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox