alliedmodders / sourcemod

SourceMod - Source Engine Scripting and Administration
http://www.sourcemod.net/
991 stars 427 forks source link

m_AllocSize corrupted on allocation failure #1233

Closed Kailo97 closed 4 years ago

Kailo97 commented 4 years ago

I'm investigated this crush https://crash.limetech.org/7luvujw7t3ni And here is 100% sure, what here is problem, because we must to get "error spam" in log, but not crush. In console we also see this error in some moments before crush (probably in previous frame)

L 04/17/2020 - 21:30:29: [SM] Exception reported: Failed to grow array
L 04/17/2020 - 21:30:29: [SM] Blaming: SurfTimer.smx
L 04/17/2020 - 21:30:29: [SM] Call stack trace:
L 04/17/2020 - 21:30:29: [SM]   [0] PushArrayArray
L 04/17/2020 - 21:30:29: [SM]   [1] Line 649, surftimer/replay/replay.sp::RecordReplay
L 04/17/2020 - 21:30:29: [SM]   [2] Line 950, surftimer/hooks/hooks.sp::OnPlayerRunCmd

https://github.com/alliedmodders/sourcemod/blob/0ac78758eb8254aed212acb44a0a9d0ad35bb0d2/core/logic/CellArray.h#L210-L241 So, i see here very important bug to fix: m_AllocSize changed before we do allocation and confirm success or failure. Steps:

State 1:
m_AllocSize = N
m_Size = N

Frame M: We tried to push (PushArrayArray -> CellArray::push (sub call to CellArray::GrowIfNeeded with count = 1) m_Size + count > m_AllocSize N + 1 > N -> dobule m_AllocSize realloc failed and return NULL Get "Failed to grow array" error

State 2:
m_AllocSize = N * 2
m_Size = N

We failed to alloc, but change allocation size!!! Frame M+1: So, we have also to push Grow will think what we have enough space to fit array, but space not accually allocated. https://github.com/alliedmodders/sourcemod/blob/0ac78758eb8254aed212acb44a0a9d0ad35bb0d2/core/logic/CellArray.h#L212-L216 So, memcopy will try to write, and here is get out of bounds.

This bug also will be on memalloc, if we failed to alloc. m_Data will be NULL, but m_AllocSize not zero for access at next time.

coverrt commented 4 years ago

This issue was resolved in the SurfTimer once the memory leak was fixed. The issue occurred when SRCDS was at 4GB memory usage which resulted in an OOM error.