From 788175e0d60b3f887b34a9ee6e0f8a51cc2ac59c Mon Sep 17 00:00:00 2001 From: Alex Orlenko Date: Fri, 4 Apr 2025 22:18:14 +0100 Subject: [PATCH] Restrict access to Luau VM from UserData destructors. It's unsafe to make almost any Lua calls when userdata destructor is running. This can cause recursive GC run and crash. See https://github.com/luau-lang/luau/pull/510 for some details. --- mlua-sys/src/lua51/lua.rs | 4 ++-- mlua-sys/src/lua52/lua.rs | 4 ++-- mlua-sys/src/lua53/lua.rs | 4 ++-- mlua-sys/src/lua54/lua.rs | 4 ++-- mlua-sys/src/luau/compat.rs | 2 +- mlua-sys/src/luau/lua.rs | 10 +++++----- src/memory.rs | 2 +- src/state.rs | 14 ++++++++++++-- src/state/extra.rs | 4 ++++ src/userdata/util.rs | 11 +++++++++-- 10 files changed, 40 insertions(+), 19 deletions(-) diff --git a/mlua-sys/src/lua51/lua.rs b/mlua-sys/src/lua51/lua.rs index 4789c56..adbcd30 100644 --- a/mlua-sys/src/lua51/lua.rs +++ b/mlua-sys/src/lua51/lua.rs @@ -84,10 +84,10 @@ pub type lua_Reader = pub type lua_Writer = unsafe extern "C-unwind" fn(L: *mut lua_State, p: *const c_void, sz: usize, ud: *mut c_void) -> c_int; -/// Type for memory-allocation functions +/// Type for memory-allocation functions (no unwinding) #[rustfmt::skip] pub type lua_Alloc = - unsafe extern "C-unwind" fn(ud: *mut c_void, ptr: *mut c_void, osize: usize, nsize: usize) -> *mut c_void; + unsafe extern "C" fn(ud: *mut c_void, ptr: *mut c_void, osize: usize, nsize: usize) -> *mut c_void; #[cfg_attr(all(windows, raw_dylib), link(name = "lua51", kind = "raw-dylib"))] extern "C-unwind" { diff --git a/mlua-sys/src/lua52/lua.rs b/mlua-sys/src/lua52/lua.rs index b77ef14..a5fed2b 100644 --- a/mlua-sys/src/lua52/lua.rs +++ b/mlua-sys/src/lua52/lua.rs @@ -89,10 +89,10 @@ pub type lua_Reader = pub type lua_Writer = unsafe extern "C-unwind" fn(L: *mut lua_State, p: *const c_void, sz: usize, ud: *mut c_void) -> c_int; -/// Type for memory-allocation functions +/// Type for memory-allocation functions (no unwinding) #[rustfmt::skip] pub type lua_Alloc = - unsafe extern "C-unwind" fn(ud: *mut c_void, ptr: *mut c_void, osize: usize, nsize: usize) -> *mut c_void; + unsafe extern "C" fn(ud: *mut c_void, ptr: *mut c_void, osize: usize, nsize: usize) -> *mut c_void; #[cfg_attr(all(windows, raw_dylib), link(name = "lua52", kind = "raw-dylib"))] extern "C-unwind" { diff --git a/mlua-sys/src/lua53/lua.rs b/mlua-sys/src/lua53/lua.rs index 3fd84f5..efb85f0 100644 --- a/mlua-sys/src/lua53/lua.rs +++ b/mlua-sys/src/lua53/lua.rs @@ -96,10 +96,10 @@ pub type lua_Reader = pub type lua_Writer = unsafe extern "C-unwind" fn(L: *mut lua_State, p: *const c_void, sz: usize, ud: *mut c_void) -> c_int; -/// Type for memory-allocation functions +/// Type for memory-allocation functions (no unwinding) #[rustfmt::skip] pub type lua_Alloc = - unsafe extern "C-unwind" fn(ud: *mut c_void, ptr: *mut c_void, osize: usize, nsize: usize) -> *mut c_void; + unsafe extern "C" fn(ud: *mut c_void, ptr: *mut c_void, osize: usize, nsize: usize) -> *mut c_void; #[cfg_attr(all(windows, raw_dylib), link(name = "lua53", kind = "raw-dylib"))] extern "C-unwind" { diff --git a/mlua-sys/src/lua54/lua.rs b/mlua-sys/src/lua54/lua.rs index afad4a3..09c7c6a 100644 --- a/mlua-sys/src/lua54/lua.rs +++ b/mlua-sys/src/lua54/lua.rs @@ -95,10 +95,10 @@ pub type lua_Reader = pub type lua_Writer = unsafe extern "C-unwind" fn(L: *mut lua_State, p: *const c_void, sz: usize, ud: *mut c_void) -> c_int; -/// Type for memory-allocation functions +/// Type for memory-allocation functions (no unwinding) #[rustfmt::skip] pub type lua_Alloc = - unsafe extern "C-unwind" fn(ud: *mut c_void, ptr: *mut c_void, osize: usize, nsize: usize) -> *mut c_void; + unsafe extern "C" fn(ud: *mut c_void, ptr: *mut c_void, osize: usize, nsize: usize) -> *mut c_void; /// Type for warning functions pub type lua_WarnFunction = unsafe extern "C-unwind" fn(ud: *mut c_void, msg: *const c_char, tocont: c_int); diff --git a/mlua-sys/src/luau/compat.rs b/mlua-sys/src/luau/compat.rs index f50c948..ebf3aea 100644 --- a/mlua-sys/src/luau/compat.rs +++ b/mlua-sys/src/luau/compat.rs @@ -357,7 +357,7 @@ pub unsafe fn luaL_loadbufferenv( fn free(p: *mut c_void); } - unsafe extern "C-unwind" fn data_dtor(_: *mut lua_State, data: *mut c_void) { + unsafe extern "C" fn data_dtor(_: *mut lua_State, data: *mut c_void) { free(*(data as *mut *mut c_char) as *mut c_void); } diff --git a/mlua-sys/src/luau/lua.rs b/mlua-sys/src/luau/lua.rs index 807682f..017ea66 100644 --- a/mlua-sys/src/luau/lua.rs +++ b/mlua-sys/src/luau/lua.rs @@ -83,12 +83,12 @@ pub type lua_Unsigned = c_uint; pub type lua_CFunction = unsafe extern "C-unwind" fn(L: *mut lua_State) -> c_int; pub type lua_Continuation = unsafe extern "C-unwind" fn(L: *mut lua_State, status: c_int) -> c_int; -/// Type for userdata destructor functions. -pub type lua_Destructor = unsafe extern "C-unwind" fn(L: *mut lua_State, *mut c_void); +/// Type for userdata destructor functions (no unwinding). +pub type lua_Destructor = unsafe extern "C" fn(L: *mut lua_State, *mut c_void); -/// Type for memory-allocation functions. +/// Type for memory-allocation functions (no unwinding). pub type lua_Alloc = - unsafe extern "C-unwind" fn(ud: *mut c_void, ptr: *mut c_void, osize: usize, nsize: usize) -> *mut c_void; + unsafe extern "C" fn(ud: *mut c_void, ptr: *mut c_void, osize: usize, nsize: usize) -> *mut c_void; /// Returns Luau release version (eg. `0.xxx`). pub const fn luau_version() -> Option<&'static str> { @@ -345,7 +345,7 @@ pub unsafe fn lua_newuserdata(L: *mut lua_State, sz: usize) -> *mut c_void { #[inline(always)] pub unsafe fn lua_newuserdata_t(L: *mut lua_State, data: T) -> *mut T { - unsafe extern "C-unwind" fn destructor(_: *mut lua_State, ud: *mut c_void) { + unsafe extern "C" fn destructor(_: *mut lua_State, ud: *mut c_void) { ptr::drop_in_place(ud as *mut T); } diff --git a/src/memory.rs b/src/memory.rs index 92c6032..ac160fe 100644 --- a/src/memory.rs +++ b/src/memory.rs @@ -96,7 +96,7 @@ impl MemoryState { } } -unsafe extern "C-unwind" fn allocator( +unsafe extern "C" fn allocator( extra: *mut c_void, ptr: *mut c_void, osize: usize, diff --git a/src/state.rs b/src/state.rs index 466009a..5f50542 100644 --- a/src/state.rs +++ b/src/state.rs @@ -1956,7 +1956,12 @@ impl Lua { #[inline(always)] pub(crate) fn lock(&self) -> ReentrantMutexGuard { - self.raw.lock() + let rawlua = self.raw.lock(); + #[cfg(feature = "luau")] + if unsafe { (*rawlua.extra.get()).running_userdata_gc } { + panic!("Luau VM is suspended while userdata destructor is running"); + } + rawlua } #[inline(always)] @@ -1983,7 +1988,12 @@ impl WeakLua { #[track_caller] #[inline(always)] pub(crate) fn lock(&self) -> LuaGuard { - LuaGuard::new(self.0.upgrade().expect("Lua instance is destroyed")) + let guard = LuaGuard::new(self.0.upgrade().expect("Lua instance is destroyed")); + #[cfg(feature = "luau")] + if unsafe { (*guard.extra.get()).running_userdata_gc } { + panic!("Luau VM is suspended while userdata destructor is running"); + } + guard } #[inline(always)] diff --git a/src/state/extra.rs b/src/state/extra.rs index d1823b5..9bea0cc 100644 --- a/src/state/extra.rs +++ b/src/state/extra.rs @@ -81,6 +81,8 @@ pub(crate) struct ExtraData { #[cfg(feature = "luau")] pub(super) interrupt_callback: Option, + #[cfg(feature = "luau")] + pub(crate) running_userdata_gc: bool, #[cfg(feature = "luau")] pub(super) sandboxed: bool, #[cfg(feature = "luau")] @@ -182,6 +184,8 @@ impl ExtraData { compiler: None, #[cfg(feature = "luau-jit")] enable_jit: true, + #[cfg(feature = "luau")] + running_userdata_gc: false, })); // Store it in the registry diff --git a/src/userdata/util.rs b/src/userdata/util.rs index 3c3b4c6..1c9275a 100644 --- a/src/userdata/util.rs +++ b/src/userdata/util.rs @@ -436,11 +436,18 @@ pub(crate) unsafe extern "C-unwind" fn collect_userdata(state: *mut ffi::lua_ // This method is called by Luau GC when it's time to collect the userdata. #[cfg(feature = "luau")] -pub(crate) unsafe extern "C-unwind" fn collect_userdata( - _state: *mut ffi::lua_State, +pub(crate) unsafe extern "C" fn collect_userdata( + state: *mut ffi::lua_State, ud: *mut std::os::raw::c_void, ) { + // Almost none Lua operations are allowed when destructor is running, + // so we need to set a flag to prevent calling any Lua functions + let extra = (*ffi::lua_callbacks(state)).userdata as *mut crate::state::ExtraData; + (*extra).running_userdata_gc = true; + // Luau does not support _any_ panics in destructors (they are declared as "C", NOT as "C-unwind"), + // so any panics will trigger `abort()`. ptr::drop_in_place(ud as *mut T); + (*extra).running_userdata_gc = false; } // This method can be called by user or Lua GC to destroy the userdata.