diff options
author | Loïc Guilloux <glx22@users.noreply.github.com> | 2021-10-02 21:08:42 +0200 |
---|---|---|
committer | GitHub <noreply@github.com> | 2021-10-02 21:08:42 +0200 |
commit | a53cfeef131fab66dc4a2395279b64706c5afed9 (patch) | |
tree | f1a0293a2cdda0d5fb651a8855a9be78926a72cd | |
parent | a2cf81e7221ec4cfd1e37b356caf59a1481cdfcf (diff) | |
download | openttd-a53cfeef131fab66dc4a2395279b64706c5afed9.tar.xz |
Fix #9548, e5fedcd: [Squirrel] Crash during engine cleanup after reaching memory limit on realloc (#9592)
-rw-r--r-- | src/script/squirrel.cpp | 25 |
1 files changed, 14 insertions, 11 deletions
diff --git a/src/script/squirrel.cpp b/src/script/squirrel.cpp index 8d2aa3b78..2485ed791 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()); |