From d6311b4e154fedec8799b6f752cfa73f2aefa176 Mon Sep 17 00:00:00 2001 From: Peter Barker Date: Sun, 23 Jun 2024 12:14:53 +1000 Subject: [PATCH] AP_Scripting: correct use-after-free in script statistics run_next_script can free the script if the script runs over-time. ... so stop using data from that freed script structure! --- libraries/AP_Scripting/lua_scripts.cpp | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/libraries/AP_Scripting/lua_scripts.cpp b/libraries/AP_Scripting/lua_scripts.cpp index b1b60a29c8..a25680adac 100644 --- a/libraries/AP_Scripting/lua_scripts.cpp +++ b/libraries/AP_Scripting/lua_scripts.cpp @@ -570,8 +570,11 @@ void lua_scripts::run(void) { if ((_debug_options.get() & uint8_t(DebugLevel::RUNTIME_MSG)) != 0) { GCS_SEND_TEXT(MAV_SEVERITY_DEBUG, "Lua: Running %s", scripts->name); } - // copy name for logging, cant do it after as script reschedule moves the pointers - const char * script_name = scripts->name; + // take a copy of the script name for the purposes of + // logging statistics. "scripts" may become invalid + // during the "run_next_script" call, below. + char script_name[128+1] {}; + strncpy_noterm(script_name, scripts->name, 128); #if DISABLE_INTERRUPTS_FOR_SCRIPT_RUN void *istate = hal.scheduler->disable_interrupts_save(); @@ -580,6 +583,10 @@ void lua_scripts::run(void) { const int startMem = lua_gc(L, LUA_GCCOUNT, 0) * 1024 + lua_gc(L, LUA_GCCOUNTB, 0); const uint32_t loadEnd = AP_HAL::micros(); + // NOTE! the base pointer of our scripts linked list, + // *and all its contents* may become invalid as part of + // "run_next_script"! So do *NOT* attempt to access + // anything that was in *scripts after this call. run_next_script(L); const uint32_t runEnd = AP_HAL::micros();