Queuestion about configENABLE_HEAP_PROTECTOR in heap_5

Kernel
1

Hi

I’m implementing heap analysis and find configENABLE_HEAP_PROTECTOR is very useful to validate free heap list items.
But when validate those allocated heap block structure, it seems that the canary is not used.
The pxNextFreeBlock of a allocated heap block structure is still NULL which not do xor with canary.

--- a/portable/MemMang/heap_5.c
+++ b/portable/MemMang/heap_5.c
@@ -333,7 +333,7 @@ void * pvPortMalloc( size_t xWantedSize )
                     /* The block is being returned - it is allocated and owned
                      * by the application and has no "next" block. */
                     heapALLOCATE_BLOCK( pxBlock );
-                    pxBlock->pxNextFreeBlock = NULL;
+                    pxBlock->pxNextFreeBlock = heapPROTECT_BLOCK_POINTER( NULL );
                     xNumberOfSuccessfulAllocations++;
                 }

How about using canary for allocated heap blocks?

Thanks.

2

Thank you for the suggestion - seems good to me. You’d need to update vPortFree where pxNextFreeBlock is used -

configASSERT( pxLink->pxNextFreeBlock == NULL ); <-- Here.

        if( heapBLOCK_IS_ALLOCATED( pxLink ) != 0 )
        {
            if( pxLink->pxNextFreeBlock == NULL ) <-- and Here.
            {

And we can make the same change in heap_4. Would you like to raise a PR?

3

Of course! I have made the changes as you said. Right now I’m testing heap_5 for full test which has been running for 4 hours and it looks good. Later I will test heap_4 and raise a PR.

4

Thank you for your contribution!

5

PR has been raised, looking forward to your response.
Thank you!

6

We merged your PR. Thank you for your contribution!

7

Hi @aggarg

Sorry to bother you. When reading the source code of heap_5, I have a question.
After call vPortDefineHeapRegions() that define multiple heap region, there is a "pxFakeEnd (yellow)“ at the bottom of each heap region. The red block is the real pxEnd. But if a block(blue) adjacent to yellow need to be released, it seems that the yellow and blue will be combined into a block.

image
image1318×596 25.3 KB

At this time, the “pxFakeEnd” no longer exit and is that the correct behavior?

8

The size of the “pxFakeEnd” is set to 0. As a result, the following check in the prvInsertBlockIntoFreeList won’t succeed (assuming that the 2 memory regions are not contiguous) -

 if( ( puc + pxBlockToInsert->xBlockSize ) == ( uint8_t * ) heapPROTECT_BLOCK_POINTER( pxIterator->pxNextFreeBlock ) )

So the two blocks wont be merged and “pxFakeEnd” will continue to exist.

9

Thanks for your reply.

  if( ( puc + pxBlockToInsert->xBlockSize ) == ( uint8_t * ) heapPROTECT_BLOCK_POINTER( pxIterator->pxNextFreeBlock ) )

The two blocks ( freed block and “pxEnd”) are not combined because the size of the “pxFakeEnd” is [0](https:// pxBlockToInsert->xBlockSize += heapPROTECT_BLOCK_POINTER( pxIterator->pxNextFreeBlock )->xBlockSize;). But I think this check will succeed and the freed block will point to next free block rather than “pxFakeEnd”.

image
image1339×523 30.7 KB

The orange block represents free block. In this case, “pxFakeEnd” is no longer in the free list, but it still exists.

10

Thank you for explaining - I understand it now. You are right but the code would still work as expected because the free list would still be continuous.

11

Thank you for confirming my understanding. I won’t raise a PR to keep “pxFakeEnd” in the free list if it dose not influence heap management.
By the way, when use configENABLE_HEAP_PROTECTOR, the macro heapVALIDATE_BLOCK_POINTER seems to have certaion limitations in heap_5.

/* Assert that a heap block pointer is within the heap bounds. */
    #define heapVALIDATE_BLOCK_POINTER( pxBlock )                       \
    configASSERT( ( pucHeapHighAddress != NULL ) &&                     \
                  ( pucHeapLowAddress != NULL ) &&                      \
                  ( ( uint8_t * ) ( pxBlock ) >= pucHeapLowAddress ) && \
                  ( ( uint8_t * ) ( pxBlock ) < pucHeapHighAddress ) )

Because heap_5 is used for multiple separated memory spaces. However, this macro only verifies the highest and lowest addresses. Because the memory space is scattered, this address may be located in a location that does not belong to an heap region, but is still between the highest and lowest addresses.
I’m trying to improve it but it seems a bit difficult.

12

Feel free to share if you come up with any suggestions!

13

Dear @aggarg
I have raised a PR, looking forward to your response.
Thank you!

14

Thank you for sharing. Will take a look.