Chris Solomon

Bug Report: Memory pool corruption in _lwmem_alloc_align_internal

Discussion created by Chris Solomon on Jul 27, 2014
Latest reply on Oct 23, 2017 by MARTIN BACHEM

Hello,

I have just tracked down a tricky bug in lwmem.c.

When an aligned allocation is made, the following process occurs:

 

1) Find a free block

2) Figure out how large a shift is required to make the allocation aligned

3) If free block isn't big enough, go back to 1

4) Create a new free block at the beginning of the free block (size is the previously determined shift).

5) Set the allocated block pointer to the location for the new allocated block

     (Sets up the free block's 'nextblock' field to point to the new allocated block, & sets the free block's size)

6a) If there is enough extra space in the allocated block, split it, creating another free block.

     (The next block is set up - size, pool, nextblock, and the allocated block has it's size set)

6b) If there isn't enough extra space in the allocated block, the allocated block *should* use the remaining space in the block.

     BUT... here is the problem. The size of the newly allocated block is not set, and is undefined.

     This causes undefined behavior (usually watchdog resets) when a task's resources are freed (for example), as the allocations in the pool cannot be 'walked'

 

This bug is present in MQX versions 4.0.1 and 4.1.0 (possibly others).

 

Here is my fix (based on the MQX 4.1.0 code) - starts in lwmem.c at line 475

 

prev_block_ptr->BLOCKSIZE = shift;
block_ptr->U.NEXTBLOCK = prev_block_ptr->U.NEXTBLOCK;


next_block_size = block_size - requested_size - shift;
if (next_block_size >= LWMEM_MIN_MEMORY_STORAGE_SIZE)
{
    /*
     * The current block is big enough to split.
     * into 2 blocks.... the part to be allocated is one block,
     * and the rest remains as a free block on the free list.
     */
    next_block_ptr = (LWMEM_BLOCK_STRUCT_PTR) (void *) ((unsigned char *) block_ptr + requested_size);
    /* Initialize the new physical block values */
    next_block_ptr->BLOCKSIZE = next_block_size;
    /* Link new block into the free list */
    next_block_ptr->POOL = (void *) mem_pool_ptr;
    next_block_ptr->U.NEXTBLOCK = block_ptr->U.NEXTBLOCK;
    block_ptr->U.NEXTBLOCK = (void *) next_block_ptr;
}
else
{
     /* Take the entire block */
     requested_size = block_size - shift;
     next_block_ptr = block_ptr->U.NEXTBLOCK;
} /* Endif */

block_ptr->BLOCKSIZE = requested_size;



 

This should have a relatively small impact - aligned allocation is used in only a couple of places.

Outcomes