mbedTLS + FreeRTOS mutex = memory leak

Partners & Sponsors AWS
1

I’m not sure if this is the right place, but maybe the mbedTLS gurus assume static mutexes so it becomes an incompatibility between mbedTLS and FreeRTOS? Otherwise it’s maybe an mbedTLS bug that I need to report elsewhere?

= mbed TLS 2.16.9 branch released 2020-12-11

ecp_mul_comb creates a mutex via mbedtls_mutex_init() which ends up being aws_mbedtls_mutex_init() which calls xQueueCreateMutex() which calls pvPortMalloc() (with many intermediaries).

Later, ecp_mul_comb calls ecp_drbg_free() which calls mbedtls_ctr_drbg_free() which deletes the mutex, but then ecp_drbg_free() immediately re-creates the mutex using mbedtls_mutex_init().

So when ecp_mul_comb exits there is still a mutex malloc’ed but not free’ed. ecp_mul_comb does return a pointer to the context which contains a pointer to the mutex but I can’t see that anybody deletes the mutex in the end.

So when I close my MQTT connection and re-open it, memory leaks.

Because all of this is surrounded by #ifndef MBEDTLS_ECP_NO_INTERNAL_RNG the problem goes away when I #define MBEDTLS_ECP_NO_INTERNAL_RNG. This was not present in the previous version of the FreeRTOS bundle I worked with, so I’m thinking this is a recent security enhancement that has not been fully tested?

Combined with my previous post, I’m now down to one malloc-without-a-free out of the four I started this week with. Now to track down the last one.

2

Please provide a link to the implementation of that function so I can understand the issue correctly. I’m not sure about your reference to static allocation.

If the mutex was created using xSemaphoreCreateMutex() and, as you say, the mutex is being deleted, then the vSemaphoreDelete() function frees the memory again automatically. The same vSemaphoreDelete() function is used to delete both statically and dynamically allocated semaphores.

3

Hi Richard

In ecp.c

static int ecp_mul_comb( ...)
{
...
#if !defined(MBEDTLS_ECP_NO_INTERNAL_RNG)
    ecp_drbg_context drbg_ctx;
    ecp_drbg_init( &drbg_ctx );
#endif
...
#if !defined(MBEDTLS_ECP_NO_INTERNAL_RNG)
    ecp_drbg_free( &drbg_ctx );
#endif
...
}

This looks good. However,

static inline void ecp_drbg_free( ecp_drbg_context *ctx )
{
    mbedtls_hmac_drbg_free( ctx );
}

And in hmac_drbg.c:

/*
 *  This function resets HMAC_DRBG context to the state immediately
 *  after initial call of mbedtls_hmac_drbg_init().
 */
void mbedtls_hmac_drbg_free( mbedtls_hmac_drbg_context *ctx )
{
    if( ctx == NULL )
        return;

#if defined(MBEDTLS_THREADING_C)
    mbedtls_mutex_free( &ctx->mutex );
#endif
    mbedtls_md_free( &ctx->md_ctx );
    mbedtls_platform_zeroize( ctx, sizeof( mbedtls_hmac_drbg_context ) );
    ctx->reseed_interval = MBEDTLS_HMAC_DRBG_RESEED_INTERVAL;
#if defined(MBEDTLS_THREADING_C)
    **mbedtls_mutex_init( &ctx->mutex );**
#endif
}

mbedtls_hmac_drbg_free() deletes the mutex and then immediately creates a new mutex which is left on the heap after I IotMqtt_Disconnect().

Thanks

W

(Reference to static, and unimportant: Presumably if mbedTLS is used with an OS that doesn’t allocate memory on the heap when creating a semaphone, this would not be an issue.)

4

This comment seems to indicate that the function is not necessarily free but re-init [An example of bad naming]. Are you using these definitions: https://github.com/aws/amazon-freertos/blob/master/libraries/freertos_plus/standard/crypto/src/iot_crypto.c#L98

Would this problem go away if you use this definition of mbedtls_threading_mutex_t and mbedtls_platform_mutex_free:

Thanks.

5

Hi Gaurav

I made the problem “go away” by enabling the NO_INTERNAL_RNG i.e. disabling the security feature introduced recently.

But the mbedTLS people need to fix the problem, naming or otherwise, and that’s why I wondered whether this is the correct place to report the issue.

Edit: OK (1) I spoke too soon, mbedtls_ctr_drbg_free is also called by TLS_Cleanup leading to another mutex being left on the stack and (2) this is fixed in the Feb 9 (Feb 22?) version of mbedTLS “Fix mutex leak in CTR_DRBG” (If I understand it right the current freeRTOS git pulls in the Dec 10 2020 version of mbedTLS).

W

6

Hi @wrm,

Thanks for this report, as you correctly point out this is resolved in mbedtls v2.16.10. I have created a pull request to update Amazon FreeRTOS.

If you are working on a released version, you can just cherry-pick the commit in when it lands.

Thanks,

Carl