From a53cfeef131fab66dc4a2395279b64706c5afed9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lo=C3=AFc=20Guilloux?= Date: Sat, 2 Oct 2021 21:08:42 +0200 Subject: [PATCH] Fix #9548, e5fedcd: [Squirrel] Crash during engine cleanup after reaching memory limit on realloc (#9592) --- src/script/squirrel.cpp | 25 ++++++++++++++----------- 1 file changed, 14 insertions(+), 11 deletions(-) diff --git a/src/script/squirrel.cpp b/src/script/squirrel.cpp index 8d2aa3b783..2485ed791a 100644 --- a/src/script/squirrel.cpp +++ b/src/script/squirrel.cpp @@ -69,19 +69,16 @@ struct ScriptAllocator { */ void CheckAllocation(size_t requested_size, void *p) { - if (this->allocated_size > this->allocation_limit && !this->error_thrown) { + if (this->allocated_size + requested_size > this->allocation_limit && !this->error_thrown) { /* Do not allow allocating more than the allocation limit, except when an error is * already as then the allocation is for throwing that error in Squirrel, the * associated stack trace information and while cleaning up the AI. */ this->error_thrown = true; char buff[128]; seprintf(buff, lastof(buff), "Maximum memory allocation exceeded by " PRINTF_SIZE " bytes when allocating " PRINTF_SIZE " bytes", - this->allocated_size - this->allocation_limit, requested_size); + this->allocated_size + requested_size - this->allocation_limit, requested_size); /* Don't leak the rejected allocation. */ free(p); - p = nullptr; - /* Allocation rejected, don't count it. */ - this->allocated_size -= requested_size; throw Script_FatalError(buff); } @@ -98,8 +95,6 @@ struct ScriptAllocator { this->error_thrown = true; char buff[64]; seprintf(buff, lastof(buff), "Out of memory. Cannot allocate " PRINTF_SIZE " bytes", requested_size); - /* Allocation failed, don't count it. */ - this->allocated_size -= requested_size; throw Script_FatalError(buff); } } @@ -107,10 +102,11 @@ struct ScriptAllocator { void *Malloc(SQUnsignedInteger size) { void *p = malloc(size); - this->allocated_size += size; this->CheckAllocation(size, p); + this->allocated_size += size; + #ifdef SCRIPT_DEBUG_ALLOCATIONS assert(p != nullptr); assert(this->allocations.find(p) == this->allocations.end()); @@ -134,14 +130,21 @@ struct ScriptAllocator { assert(this->allocations[p] == oldsize); this->allocations.erase(p); #endif + /* Can't use realloc directly because memory limit check. + * If memory exception is thrown, the old pointer is expected + * to be valid for engine cleanup. + */ + void *new_p = malloc(size); - void *new_p = realloc(p, size); + this->CheckAllocation(size - oldsize, new_p); + + /* Memory limit test passed, we can copy data and free old pointer. */ + memcpy(new_p, p, std::min(oldsize, size)); + free(p); this->allocated_size -= oldsize; this->allocated_size += size; - this->CheckAllocation(size, p); - #ifdef SCRIPT_DEBUG_ALLOCATIONS assert(new_p != nullptr); assert(this->allocations.find(p) == this->allocations.end());