[heap_4.c] data access violation when inserting the block with the highest memory address into the free list

mcinnis wrote on Monday, March 26, 2018:

Hi,

I hit a problem of data access violation when inserting a block whoms address is the highest in the memory. The following interation of current free list will go to the end of block with the condition, pxIterator->pxNextFreeBlock = NULL, and then the next iteration will trigger illegal memory access, NULL->pxNextFreeBlock.

	/* Iterate through the list until a block is found that has a higher address
	than the block being inserted. */
	for( pxIterator = &xStart; pxIterator->pxNextFreeBlock < pxBlockToInsert; pxIterator = pxIterator->pxNextFreeBlock )
	{
		/* Nothing to do here, just iterate to the right position. */
	}

I tried to fix it with following snippet of code, and it works.

--- a/kernel/FreeRTOS/Source/portable/MemMang/heap_4.c
+++ b/kernel/FreeRTOS/Source/portable/MemMang/heap_4.c
@@ -443,7 +443,9 @@ uint8_t *puc;
        than the block being inserted. */
        for( pxIterator = &xStart; pxIterator->pxNextFreeBlock < pxBlockToInsert; pxIterator = pxIterator->pxNextFreeBlock )
        {
-               /* Nothing to do here, just iterate to the right position. */
+               /* pxIterator is the block with highest address in the free list */
+               if(pxIterator->pxNextFreeBlock == NULL)
+                       break;
        }

        /* Do the block being inserted, and the block it is being inserted after
@@ -462,7 +464,8 @@ uint8_t *puc;
        /* Do the block being inserted, and the block it is being inserted before
        make a contiguous block of memory? */
        puc = ( uint8_t * ) pxBlockToInsert;
-       if( ( puc + pxBlockToInsert->xBlockSize ) == ( uint8_t * ) pxIterator->pxNextFreeBlock )
+       if( pxIterator->pxNextFreeBlock && /* if NULL, there is no block being inserted before */
+               ( puc + pxBlockToInsert->xBlockSize ) == ( uint8_t * ) pxIterator->pxNextFreeBlock )
        {
                if( pxIterator->pxNextFreeBlock != pxEnd )
                {

Could anyone help to double confirm this?

rtel wrote on Monday, March 26, 2018:

I would be grateful if you can provide more information to enable us to
try and replicate the issue you are seeing. In particular, what do you
mean by “inserting a block whoms address is the highest in the memory”?
How can I replicate that?

For example, if all my heap is free, then I call pvPortMalloc() three
times, each time to obtain one third of the available heap memory, the
memory returned to me the lowest one third of the heap for the first
pvPortMalloc() call, the middle third for the second pvPortMalloc()
call, and the top third for the third pvPortMalloc() call. After this,
is calling vPortFree() to return the highest of those three blocks what
you mean by “inserting a block whose address is the highest in the memory”?

Hi
I’m not sure if this fix relates to my problem i’ve seeing.

My pxBlockToInsert->pxNextFreeBlock is NULL and so it’s stuck there

The Starting address of the heap is 0x20019094 and i got 120000bytes so that goes up to 0x20036554.

pxBlockToInsert is at 0x2002C428
pxIterator is at 0x2002C400

so it does not seem to be at end as the first post saying indends to fix.
xPortGetFreeHeapSize was called few ms before and gives 36k back.

i now try to hack those fixes in and check if it helps.
May someone look at it, and fix it in the code, since this is still in there…

Cheers

Hi @saperlot
pxBlockToInsert->pxNextFreeBlock should never be null, as there is an end marker pxEnd.
Any nextFreeBlock should not be null except the memory that is not free/used, which no pointer should point to it from the internal data structures, it is the responsibility of the user to keep track of those addresses.

We would be grateful if you can provide us with detailed steps to reproduce what you are seeing.

Thanks

Hi @gedeonag ,
I debuged it some more here are more information…

So, some starting information.
Using Heap4 with #define configTOTAL_HEAP_SIZE ((size_t)120000)
uCHeap is at 0x200190A4 and goes up to 0x20036564 This is on RAM which goes up to `0x2003BFFF.

i try to show the call stack here:

  • xTaskCreate gets called with usStackDepth = 0x100
  • pvPortMallocgets called with xWantedSize = 0x400 (correct)
  • xFreeBytesRemaining is 0xDEE0 so it goes on
  • xStart is at 0x20036DE0 pxNextFreeBlock points to 0x200283A0
  • this gets pxBlock = xStart.pxNextFreeBlock;
  • now it cycles through the list to find a block. it gets a new one at 0x20028700
  • then it creates a new Block with block size = pxNewBlockLink->xBlockSize = pxBlock->xBlockSize - xWantedSize; BUt pxBlock->xBlockSize is = 0x80000050 ?!!! so the newblock size gets to 0x7FFFFC48.
  • then it calls prvInsertBlockIntoFreeList and
    iterates with this loop: for( pxIterator = &xStart; pxIterator->pxNextFreeBlock < pxBlockToInsert; pxIterator = pxIterator->pxNextFreeBlock ) after 1 call pxIterator turns NULL and iterates forever .

So i can confirm that the solution up there fixes the problem… But i have to admit i did not dive into it very deeply. i have seen that this block size gets weird and then the next call to malloc wont work anymore,

Update: it does not solve it completely, calling vPortGetHeapStats gets a hardfault. It cycles while( pxBlock != pxEnd ); pxEnd is at 0x20036570 but at block 0x20029E40 pxNextFreeblock points to 0x6c472065 (crap) and boom → hardfault…

Thanks for this detail - it helps a lot. It looks like the issue occurs in step 7 of your bullet list. We will re-create this scenario to see if we can see how that could happen.

Which MCU are you using (relevant to see the alignment requirements and word size)?

[addition]
Also - which version of FreeRTOS are you using?
[\addition]

Hi @saperlot,

Thanks for the extra information!

I went through the number you provided, and I don’t see how it would be possible in the code for the value of pxBlockSize to be 0x80000050. ( using the latest version )

One of the explanations I could come up with, is that you might have some buffer overflow in your code that is getting the block-size to be fully or partially overridden.

1- Could you check the buffer you received by malloc which have the first addresses below this address, and make sure the buffer is not getting overflowed ?

2- If you are using a debugger you could insert a watchpoint on that memory address and let the program run

3- Try to memset all the ucHeap to zero or some known set of values, would you see a different result? (to make sure the pxBlock is not pointing to some random location, could be xStart getting overridden, or pxEnd in some different cases)

4- If nothing of the above works, it would be beneficial to isolate the issue by trying/providing a program that just does the allocation and deallocation to hit that problem.

Hope this helps.

Hi,
I really appreciate your help. I choose number 1 as a hit in the bullseye. I indeed found a portion where i get a strlen buffer and strcpy this into. So it overwrites the buffer by 1 byte. So, this shows again that the weirdest errors or bugs are to a high probability just simple bullshit failures…

I’m really sorry to have stolen your time, and i really reallyy appreciate your great help…

buffer overflow could also be @mcinnis fault. I mean, it has worked with this workaround but failed in getting the stats…
Cheers

Hi @saperlot,

Thats why we are here for, the most important thing is that your problem is solved :slight_smile:

I believe so as well. The workaround could be hiding a more serious problem behind it, that could cause some issues down the line.

Cheers,
Alfred