Skip to content

m_AllocSize corrupted on allocation failure #1233

@Kailo97

Description

@Kailo97

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

bool GrowIfNeeded(size_t count)
{
/* Shortcut out if we can store this */
if (m_Size + count <= m_AllocSize)
{
return true;
}
/* Set a base allocation size of 8 items */
if (!m_AllocSize)
{
m_AllocSize = 8;
}
/* If it's not enough, keep doubling */
while (m_Size + count > m_AllocSize)
{
m_AllocSize *= 2;
}
/* finally, allocate the new block */
if (m_Data)
{
cell_t *data = static_cast<cell_t*>(realloc(m_Data, sizeof(cell_t) * m_BlockSize * m_AllocSize));
if (!data) // allocation failure
{
return false;
}
m_Data = data;
} else {
m_Data = static_cast<cell_t*>(malloc(sizeof(cell_t) * m_BlockSize * m_AllocSize));
}
return (m_Data != nullptr);
}

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.

/* Shortcut out if we can store this */
if (m_Size + count <= m_AllocSize)
{
return true;
}

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.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions