mirror of
https://github.com/ArduPilot/ardupilot.git
synced 2026-02-08 19:39:51 +08:00
AP_Scripting: run scripting engine in protected mode
Lua calls our registered panic function if a Lua error is raised outside of any protected mode set up by `lua_pcall` and friends. As scripts themselves are run in protected mode, such an error will only originate from our engine code. The current code handles panics, but does so incorrectly by neglecting to free the error message buffer during a panic exit. As the underlying heap is eventually destroyed and recreated, that static buffer pointer is to a heap that no longer exists. If the new state raises an error it will try to free that pointer, causing heap corruption and sadness. Fix this issue and avoid similar ones by refactoring to run the main part of the scripting engine in Lua protected mode. This creates exactly one exit path from the scripting engine and avoids the need for fragile panic handling infrastructure that duplicates what is built into Lua. Quoth the Lua manual, "The panic function, as its name implies, is a mechanism of last resort. Programs should avoid it." This is unlikely to be triggerable in flight as the main engine loop does not appear to use the Lua API in a way which can trigger errors. But the possibility can't be fully excluded. It is, however, possible to trigger beforehand using a perfectly wrong heap size that causes a memory error during initialization. Note that due to the error message buffer being freed properly now, an error message originating from the engine will not cause a pre-arm failure. This could be improved in the future.
This commit is contained in:
committed by
Thomas Watson
parent
24d05a63eb
commit
67860f11c0
@@ -501,9 +501,49 @@ void lua_scripts::run(void) {
|
||||
GCS_SEND_TEXT(MAV_SEVERITY_CRITICAL, "Lua: Couldn't allocate a lua state");
|
||||
return;
|
||||
}
|
||||
// initialize the state with a pointer to us for ls_* callback trampolines
|
||||
// (see ls_object_from_state)
|
||||
*static_cast<lua_scripts**>(lua_getextraspace(L)) = this;
|
||||
|
||||
lua_atpanic(L, atpanic);
|
||||
|
||||
#ifndef __clang_analyzer__
|
||||
succeeded_initial_load = true;
|
||||
#endif // __clang_analyzer__
|
||||
|
||||
|
||||
// call main engine function in protected mode now that Lua itself is ready.
|
||||
// this catches any errors raised by the code between here and Lua scripts.
|
||||
// our current function must not use any Lua API which can raise an error!
|
||||
lua_pushcfunction(L, &ls_run_engine);
|
||||
if (lua_pcall(L, 0, 0, 0) != LUA_OK) { // no args, returns, or msg handler
|
||||
set_and_print_new_error_message(MAV_SEVERITY_CRITICAL,
|
||||
"Engine Error: %s", get_error_object_message(L));
|
||||
}
|
||||
// we are now finished with Lua, tear everything down
|
||||
|
||||
if (lua_state != nullptr) {
|
||||
lua_close(lua_state); // shutdown the old state
|
||||
lua_state = nullptr;
|
||||
}
|
||||
|
||||
// make sure all scripts have been removed
|
||||
while (scripts != nullptr) {
|
||||
remove_script(lua_state, scripts);
|
||||
}
|
||||
|
||||
error_msg_buf_sem.take_blocking();
|
||||
if (error_msg_buf != nullptr) {
|
||||
_heap.deallocate(error_msg_buf);
|
||||
error_msg_buf = nullptr;
|
||||
}
|
||||
error_msg_buf_sem.give();
|
||||
}
|
||||
|
||||
int lua_scripts::run_engine(lua_State *L) {
|
||||
// run our scripting engine now that the Lua state is initialized. we are in
|
||||
// Lua protected mode and can safely call functions that may raise errors.
|
||||
|
||||
// set up string metatable. we set up one for all scripts that no script has
|
||||
// access to, as it's impossible to set up one per-script and we don't want
|
||||
// any script to be able to mess with it.
|
||||
@@ -537,10 +577,6 @@ void lua_scripts::run(void) {
|
||||
GCS_SEND_TEXT(MAV_SEVERITY_CRITICAL, "Lua: All directory's disabled see SCR_DIR_DISABLE");
|
||||
}
|
||||
|
||||
#ifndef __clang_analyzer__
|
||||
succeeded_initial_load = true;
|
||||
#endif // __clang_analyzer__
|
||||
|
||||
uint32_t expansion_size = 0;
|
||||
|
||||
while (AP_Scripting::get_singleton()->should_run()) {
|
||||
@@ -630,22 +666,7 @@ void lua_scripts::run(void) {
|
||||
}
|
||||
}
|
||||
|
||||
// make sure all scripts have been removed
|
||||
while (scripts != nullptr) {
|
||||
remove_script(lua_state, scripts);
|
||||
}
|
||||
|
||||
if (lua_state != nullptr) {
|
||||
lua_close(lua_state); // shutdown the old state
|
||||
lua_state = nullptr;
|
||||
}
|
||||
|
||||
error_msg_buf_sem.take_blocking();
|
||||
if (error_msg_buf != nullptr) {
|
||||
_heap.deallocate(error_msg_buf);
|
||||
error_msg_buf = nullptr;
|
||||
}
|
||||
error_msg_buf_sem.give();
|
||||
return 0; // no results
|
||||
}
|
||||
|
||||
// Return the file checksums of running and loaded scripts
|
||||
|
||||
Reference in New Issue
Block a user