Tarantool development patches archive
 help / color / mirror / Atom feed
* [Tarantool-patches] [PATCH luajit] Correctly close VM state after early OOM during open.
@ 2025-08-19  7:40 Sergey Kaplun via Tarantool-patches
  2025-09-09  8:22 ` Sergey Bronnikov via Tarantool-patches
  0 siblings, 1 reply; 4+ messages in thread
From: Sergey Kaplun via Tarantool-patches @ 2025-08-19  7:40 UTC (permalink / raw)
  To: Sergey Bronnikov; +Cc: tarantool-patches

From: Mike Pall <mike>

Reported by Assumeru.

(cherry picked from commit 5ca25ee83ec1b0343556cd5783ade449676b4037)

`lua_newstate()` sets `g->str.mask` to `~(MSize)0` before calling
`cpluaopen(). If OOM happens before the `lj_str_init()` call,
`lj_gc_freeall()` calls `gc_sweepstr()` in a loop with incorrect top
limit `g->str.mask`, which leads to the crash.

This patch changes the order of the loop iteration with the correct
bottom limit.

Sergey Kaplun:
* added the description and the test for the problem

Part of tarantool/tarantool#11691
---

Branch: https://github.com/tarantool/luajit/tree/skaplun/lj-1248-close-state-early-OOM
Related issues:
* https://github.com/tarantool/tarantool/issues/11691
* https://github.com/LuaJIT/LuaJIT/issues/1248
* https://github.com/LuaJIT/LuaJIT/issues/1311

Note: The test works for the GC64 build only, since we can't set a
custom allocator for the non-GC64 LJ_64 build. Also, to avoid failures
related to the lj-1311 the !LJ_NO_UNWIND builds are disabled.

 src/lj_gc.c                                   |  5 +-
 .../lj-1248-close-state-early-OOM.test.c      | 71 +++++++++++++++++++
 2 files changed, 73 insertions(+), 3 deletions(-)
 create mode 100644 test/tarantool-c-tests/lj-1248-close-state-early-OOM.test.c

diff --git a/src/lj_gc.c b/src/lj_gc.c
index f455b55b..3142482f 100644
--- a/src/lj_gc.c
+++ b/src/lj_gc.c
@@ -598,12 +598,11 @@ void lj_gc_finalize_cdata(lua_State *L)
 /* Free all remaining GC objects. */
 void lj_gc_freeall(global_State *g)
 {
-  MSize i, strmask;
+  MSize i;
   /* Free everything, except super-fixed objects (the main thread). */
   g->gc.currentwhite = LJ_GC_WHITES | LJ_GC_SFIXED;
   gc_fullsweep(g, &g->gc.root);
-  strmask = g->strmask;
-  for (i = 0; i <= strmask; i++)  /* Free all string hash chains. */
+  for (i = g->strmask; i != ~(MSize)0; i--)  /* Free all string hash chains. */
     gc_fullsweep(g, &g->strhash[i]);
 }
 
diff --git a/test/tarantool-c-tests/lj-1248-close-state-early-OOM.test.c b/test/tarantool-c-tests/lj-1248-close-state-early-OOM.test.c
new file mode 100644
index 00000000..6c9cb2ca
--- /dev/null
+++ b/test/tarantool-c-tests/lj-1248-close-state-early-OOM.test.c
@@ -0,0 +1,71 @@
+#include "lua.h"
+/* XXX: The "lj_arch.h" header is included for the skipcond. */
+#include "lj_arch.h"
+
+#include "test.h"
+
+#include <stdlib.h>
+
+/*
+ * LuaJIT requires at least 12000 something bytes for initial
+ * allocations. The `GG_State` requires a little bit more than
+ * 6000 bytes (around 3000 bytes is the `jit_State`).
+ */
+
+/* Currently allocated Lua memory and its limit. */
+static size_t current_memory = 0;
+const size_t memory_limit = 7000;
+
+void *limited_alloc_f(void *msp, void *ptr, size_t osize, size_t nsize)
+{
+	void *ret_ptr = NULL;
+	/* Overflow is OK here. */
+	const size_t requested_diff = nsize - osize;
+	(void)msp;
+
+	if (current_memory + requested_diff > memory_limit)
+		return NULL;
+
+	if (nsize == 0) {
+		free(ptr);
+		current_memory -= osize;
+	} else if (ptr == NULL) {
+		ret_ptr = malloc(nsize);
+		current_memory += ret_ptr ? nsize : 0;
+	} else {
+		ret_ptr = realloc(ptr, nsize);
+		current_memory += ret_ptr ? requested_diff : 0;
+	}
+	return ret_ptr;
+}
+
+static int limited_memory_on_lua_newstate(void *test_state)
+{
+	(void)test_state;
+#if LJ_64 && !LJ_GC64
+	(void)limited_alloc_f;
+	return skip("Can't use custom allocator for 64-bit host without GC64");
+#else
+	/*
+	 * Check that there is no crash and the limit is small enough.
+	 */
+	lua_State *L = lua_newstate(limited_alloc_f, NULL);
+	assert_true(L == NULL);
+	return TEST_EXIT_SUCCESS;
+#endif
+}
+
+#ifndef LJ_NO_UNWIND
+#  define LJ_NO_UNWIND 0
+#endif
+
+int main(void)
+{
+	/* See https://github.com/LuaJIT/LuaJIT/issues/1311. */
+	if (!LJ_NO_UNWIND)
+		return skip_all("Disabled for external unwinding build due to #1311");
+	const struct test_unit tgroup[] = {
+		test_unit_def(limited_memory_on_lua_newstate),
+	};
+	return test_run_group(tgroup, NULL);
+}
-- 
2.50.1


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

* Re: [Tarantool-patches] [PATCH luajit] Correctly close VM state after early OOM during open.
  2025-08-19  7:40 [Tarantool-patches] [PATCH luajit] Correctly close VM state after early OOM during open Sergey Kaplun via Tarantool-patches
@ 2025-09-09  8:22 ` Sergey Bronnikov via Tarantool-patches
  2025-09-09  9:44   ` Sergey Kaplun via Tarantool-patches
  0 siblings, 1 reply; 4+ messages in thread
From: Sergey Bronnikov via Tarantool-patches @ 2025-09-09  8:22 UTC (permalink / raw)
  To: Sergey Kaplun; +Cc: tarantool-patches

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

Hi, Sergey,

thanks for the patch! LGTM with a minor comment.

Sergey

On 8/19/25 10:40, Sergey Kaplun wrote:
> From: Mike Pall <mike>
>
> Reported by Assumeru.
>
> (cherry picked from commit 5ca25ee83ec1b0343556cd5783ade449676b4037)
>
> `lua_newstate()` sets `g->str.mask` to `~(MSize)0` before calling
> `cpluaopen(). If OOM happens before the `lj_str_init()` call,
> `lj_gc_freeall()` calls `gc_sweepstr()` in a loop with incorrect top
> limit `g->str.mask`, which leads to the crash.
>
> This patch changes the order of the loop iteration with the correct
> bottom limit.
>
> Sergey Kaplun:
> * added the description and the test for the problem
>
> Part of tarantool/tarantool#11691
> ---
>
> Branch:https://github.com/tarantool/luajit/tree/skaplun/lj-1248-close-state-early-OOM
> Related issues:
> *https://github.com/tarantool/tarantool/issues/11691
> *https://github.com/LuaJIT/LuaJIT/issues/1248
> *https://github.com/LuaJIT/LuaJIT/issues/1311
>
> Note: The test works for the GC64 build only, since we can't set a
> custom allocator for the non-GC64 LJ_64 build. Also, to avoid failures
> related to the lj-1311 the !LJ_NO_UNWIND builds are disabled.

Just for the record: CMake configuration for reproducing the issue:

CFLAGS="-DLJ_NO_UNWIND" cmake -DCMAKE_C_COMPILER=clang 
-DCMAKE_CXX_COMPILER=clang++ -S . -B build -DCMAKE_BUILD_TYPE=Debug 
-DLUA_USE_ASSERT=ON -DLUA_USE_APICHECK=ON -DLUAJIT_ENABLE_GC64=ON

>
>   src/lj_gc.c                                   |  5 +-
>   .../lj-1248-close-state-early-OOM.test.c      | 71 +++++++++++++++++++
>   2 files changed, 73 insertions(+), 3 deletions(-)
>   create mode 100644 test/tarantool-c-tests/lj-1248-close-state-early-OOM.test.c
>
> diff --git a/src/lj_gc.c b/src/lj_gc.c
> index f455b55b..3142482f 100644
> --- a/src/lj_gc.c
> +++ b/src/lj_gc.c
> @@ -598,12 +598,11 @@ void lj_gc_finalize_cdata(lua_State *L)
>   /* Free all remaining GC objects. */
>   void lj_gc_freeall(global_State *g)
>   {
> -  MSize i, strmask;
> +  MSize i;
>     /* Free everything, except super-fixed objects (the main thread). */
>     g->gc.currentwhite = LJ_GC_WHITES | LJ_GC_SFIXED;
>     gc_fullsweep(g, &g->gc.root);
> -  strmask = g->strmask;
> -  for (i = 0; i <= strmask; i++)  /* Free all string hash chains. */
> +  for (i = g->strmask; i != ~(MSize)0; i--)  /* Free all string hash chains. */
>       gc_fullsweep(g, &g->strhash[i]);
>   }
>   
> diff --git a/test/tarantool-c-tests/lj-1248-close-state-early-OOM.test.c b/test/tarantool-c-tests/lj-1248-close-state-early-OOM.test.c
> new file mode 100644
> index 00000000..6c9cb2ca
> --- /dev/null
> +++ b/test/tarantool-c-tests/lj-1248-close-state-early-OOM.test.c
> @@ -0,0 +1,71 @@
> +#include "lua.h"
> +/* XXX: The "lj_arch.h" header is included for the skipcond. */
> +#include "lj_arch.h"
> +
> +#include "test.h"
> +
> +#include <stdlib.h>
> +
> +/*
> + * LuaJIT requires at least 12000 something bytes for initial
> + * allocations. The `GG_State` requires a little bit more than
> + * 6000 bytes (around 3000 bytes is the `jit_State`).
> + */
> +
> +/* Currently allocated Lua memory and its limit. */
> +static size_t current_memory = 0;
> +const size_t memory_limit = 7000;
> +
> +void *limited_alloc_f(void *msp, void *ptr, size_t osize, size_t nsize)
> +{
> +	void *ret_ptr = NULL;
> +	/* Overflow is OK here. */
> +	const size_t requested_diff = nsize - osize;
> +	(void)msp;
> +
> +	if (current_memory + requested_diff > memory_limit)
> +		return NULL;
> +
> +	if (nsize == 0) {
> +		free(ptr);
> +		current_memory -= osize;
> +	} else if (ptr == NULL) {
> +		ret_ptr = malloc(nsize);
> +		current_memory += ret_ptr ? nsize : 0;
> +	} else {
> +		ret_ptr = realloc(ptr, nsize);
> +		current_memory += ret_ptr ? requested_diff : 0;
> +	}
> +	return ret_ptr;
> +}
> +
> +static int limited_memory_on_lua_newstate(void *test_state)
> +{
> +	(void)test_state;
> +#if LJ_64 && !LJ_GC64
> +	(void)limited_alloc_f;
> +	return skip("Can't use custom allocator for 64-bit host without GC64");
> +#else
> +	/*
> +	 * Check that there is no crash and the limit is small enough.
> +	 */
> +	lua_State *L = lua_newstate(limited_alloc_f, NULL);
s/lua_State/const lua_State/
> +	assert_true(L == NULL);
> +	return TEST_EXIT_SUCCESS;
> +#endif
> +}
> +
> +#ifndef LJ_NO_UNWIND
> +#  define LJ_NO_UNWIND 0
> +#endif
> +
> +int main(void)
> +{
> +	/* Seehttps://github.com/LuaJIT/LuaJIT/issues/1311. */
> +	if (!LJ_NO_UNWIND)
> +		return skip_all("Disabled for external unwinding build due to #1311");
> +	const struct test_unit tgroup[] = {
> +		test_unit_def(limited_memory_on_lua_newstate),
> +	};
> +	return test_run_group(tgroup, NULL);
> +}

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

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

* Re: [Tarantool-patches] [PATCH luajit] Correctly close VM state after early OOM during open.
  2025-09-09  9:44   ` Sergey Kaplun via Tarantool-patches
@ 2025-09-09  9:43     ` Sergey Bronnikov via Tarantool-patches
  0 siblings, 0 replies; 4+ messages in thread
From: Sergey Bronnikov via Tarantool-patches @ 2025-09-09  9:43 UTC (permalink / raw)
  To: Sergey Kaplun; +Cc: tarantool-patches

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

LGTM, thanks!

On 9/9/25 12:44, Sergey Kaplun wrote:
> Hi, Sergey!
> Thanks for the review!
> Fixed your comment and force-pushed the branch.
>
> On 09.09.25, Sergey Bronnikov wrote:
>> Hi, Sergey,
>>
>> thanks for the patch! LGTM with a minor comment.
>>
>> Sergey
>>
>> On 8/19/25 10:40, Sergey Kaplun wrote:
> <snipped>
>
>>> +	/*
>>> +	 * Check that there is no crash and the limit is small enough.
>>> +	 */
>>> +	lua_State *L = lua_newstate(limited_alloc_f, NULL);
>> s/lua_State/const lua_State/
> Fixed, thanks!
>
> ===================================================================
> diff --git a/test/tarantool-c-tests/lj-1248-close-state-early-OOM.test.c b/test/tarantool-c-tests/lj-1248-close-state-early-OOM.test.c
> index 6c9cb2ca..fe91d5e9 100644
> --- a/test/tarantool-c-tests/lj-1248-close-state-early-OOM.test.c
> +++ b/test/tarantool-c-tests/lj-1248-close-state-early-OOM.test.c
> @@ -49,7 +49,7 @@ static int limited_memory_on_lua_newstate(void *test_state)
>           /*
>            * Check that there is no crash and the limit is small enough.
>            */
> -        lua_State *L = lua_newstate(limited_alloc_f, NULL);
> +        const lua_State *L = lua_newstate(limited_alloc_f, NULL);
>           assert_true(L == NULL);
>           return TEST_EXIT_SUCCESS;
>   #endif
> ===================================================================
>
>>> +	assert_true(L == NULL);
>>> +	return TEST_EXIT_SUCCESS;
>>> +#endif
>>> +}
>>> +
>>> +#ifndef LJ_NO_UNWIND
>>> +#  define LJ_NO_UNWIND 0
>>> +#endif
>>> +
>>> +int main(void)
>>> +{
>>> +	/*Seehttps://github.com/LuaJIT/LuaJIT/issues/1311. */
>>> +	if (!LJ_NO_UNWIND)
>>> +		return skip_all("Disabled for external unwinding build due to #1311");
>>> +	const struct test_unit tgroup[] = {
>>> +		test_unit_def(limited_memory_on_lua_newstate),
>>> +	};
>>> +	return test_run_group(tgroup, NULL);
>>> +}

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

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

* Re: [Tarantool-patches] [PATCH luajit] Correctly close VM state after early OOM during open.
  2025-09-09  8:22 ` Sergey Bronnikov via Tarantool-patches
@ 2025-09-09  9:44   ` Sergey Kaplun via Tarantool-patches
  2025-09-09  9:43     ` Sergey Bronnikov via Tarantool-patches
  0 siblings, 1 reply; 4+ messages in thread
From: Sergey Kaplun via Tarantool-patches @ 2025-09-09  9:44 UTC (permalink / raw)
  To: Sergey Bronnikov; +Cc: tarantool-patches

Hi, Sergey!
Thanks for the review!
Fixed your comment and force-pushed the branch.

On 09.09.25, Sergey Bronnikov wrote:
> Hi, Sergey,
> 
> thanks for the patch! LGTM with a minor comment.
> 
> Sergey
> 
> On 8/19/25 10:40, Sergey Kaplun wrote:

<snipped>

> > +	/*
> > +	 * Check that there is no crash and the limit is small enough.
> > +	 */
> > +	lua_State *L = lua_newstate(limited_alloc_f, NULL);
> s/lua_State/const lua_State/

Fixed, thanks!

===================================================================
diff --git a/test/tarantool-c-tests/lj-1248-close-state-early-OOM.test.c b/test/tarantool-c-tests/lj-1248-close-state-early-OOM.test.c
index 6c9cb2ca..fe91d5e9 100644
--- a/test/tarantool-c-tests/lj-1248-close-state-early-OOM.test.c
+++ b/test/tarantool-c-tests/lj-1248-close-state-early-OOM.test.c
@@ -49,7 +49,7 @@ static int limited_memory_on_lua_newstate(void *test_state)
         /*
          * Check that there is no crash and the limit is small enough.
          */
-        lua_State *L = lua_newstate(limited_alloc_f, NULL);
+        const lua_State *L = lua_newstate(limited_alloc_f, NULL);
         assert_true(L == NULL);
         return TEST_EXIT_SUCCESS;
 #endif
===================================================================

> > +	assert_true(L == NULL);
> > +	return TEST_EXIT_SUCCESS;
> > +#endif
> > +}
> > +
> > +#ifndef LJ_NO_UNWIND
> > +#  define LJ_NO_UNWIND 0
> > +#endif
> > +
> > +int main(void)
> > +{
> > +	/* Seehttps://github.com/LuaJIT/LuaJIT/issues/1311. */
> > +	if (!LJ_NO_UNWIND)
> > +		return skip_all("Disabled for external unwinding build due to #1311");
> > +	const struct test_unit tgroup[] = {
> > +		test_unit_def(limited_memory_on_lua_newstate),
> > +	};
> > +	return test_run_group(tgroup, NULL);
> > +}

-- 
Best regards,
Sergey Kaplun

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

end of thread, other threads:[~2025-09-09  9:43 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2025-08-19  7:40 [Tarantool-patches] [PATCH luajit] Correctly close VM state after early OOM during open Sergey Kaplun via Tarantool-patches
2025-09-09  8:22 ` Sergey Bronnikov via Tarantool-patches
2025-09-09  9:44   ` Sergey Kaplun via Tarantool-patches
2025-09-09  9:43     ` Sergey Bronnikov 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