Memory leak during nbns/llmnr hostname resolution

Libraries
#1

Hello,

I was experiencing a memory leak during nbns/llmnr host name resolution of my device. I found out that the reply message to the resolve request is used with a parameter not to free the descriptor after sending it . Why should the memory for a dns reply not be freed after sending it?

Thats the function:
vReturnEthernetFrame( pxNetworkBuffer, pdFALSE );

Called in prvReplyDNSMessage(…) of the source file FreeRTOS_DNS.c.

To solve the issue I set pdFALSE to pdTRUE. So my output function will free the buffer descriptor.

#2

@niconrok : thank you very much for reporting this!

I can not replicate the problem with LLMNR, but I do see a buffer leak when the application replies positively to an NBNS request.

I do use NBNS, but I never saw any leak. The reason is that I use mostly BufferAllocation_1.c.
Lesson learned: I also test with BufferAllocation_2.c!

Here you find PR #215: Repair buffer leak when replying to NBNS requests.

You will find the new repaired FreeRTOS_DNS.c here. Maybe you want to try it out? Thanks

#3

Thank you! That solves also the problem. I will have a look on mDNS since LLMNR is not recommended for security reasons.

#4

You wrote:

vReturnEthernetFrame( pxNetworkBuffer, pdFALSE );

To solve the issue I set pdFALSE to pdTRUE.

And also:

That solves also the problem

Your proposed change will not work properly in projects where BufferAllocation_1.c is used. Explanation:

  • BufferAllocation_1.c

When a LLMNR or NBNS request must be answered, the current network buffer is re-used to send a reply. Later on, the network buffer is released by the caller.

  • BufferAllocation_2.c

The reply is longer than the request, and the network buffer is just big enough to hold the request. In order to reply, a new network buffer must be created, which is a bit longer.
This new buffer is called pxNewBuffer, and must be release after use ( in two locations, LLMNR and NBNS ):

if( pxNewBuffer != NULL )
{
    vReleaseNetworkBufferAndDescriptor( pxNewBuffer );
}
#5

I understand, thanks a lot for your proper explantion. I will do the change accordingly to yours. Altough I don’t use BufferAllocation_1.c it is much better to keep it compatible to the other allocation styles, like your way.

#6

I would be grateful if you can report back if it works well for you ( while using BufferAllocation_2.c ). Always good to have multiple testers.

#7

Just checked it but pxNewBuffer is out of scope.

To get it into the same scope I would declare pxNewBuffer at line 1816. Tested it and it works.

#8

The PR that was created by Hein was merged in the FreeRTOS+TCP repository.

Thanks for pointing this out @niconrok.