commit b15832160b220124af8d8a29336ff2b5520e5bd6
author Gilles Peskine <Gilles.Peskine@arm.com> Mon Feb 22 21:26:54 2021 +0100
committer Gilles Peskine <Gilles.Peskine@arm.com> Tue Feb 23 11:27:03 2021 +0100
tree d784d2976b7b386a73e52c5444ae88518509985c
parent 3d979f781e7d49810b6a72f41a18c7347b624361

Make entropy double-free work

Although the library documentation does not guarantee that calling
mbedtls_entropy_free() twice works, it's a plausible assumption and it's
natural to write code that frees an object twice. While this is uncommon for
an entropy context, which is usually a global variable, it came up in our
own unit tests (random_twice tests in test_suite_random).

Announce this in the same changelog entry as for RSA because it's the same
bug in the two modules.

Signed-off-by: Gilles Peskine <Gilles.Peskine@arm.com>

ChangeLog.d/rsa-mutex.txt

diff --git a/ChangeLog.d/rsa-mutex.txt b/ChangeLog.d/rsa-mutex.txt
index 49f1a84..2a477a9 100644
--- a/ChangeLog.d/rsa-mutex.txt
+++ b/ChangeLog.d/rsa-mutex.txt
@@ -1,8 +1,8 @@
 Bugfix
-   * Ensure that calling mbedtls_rsa_free() twice is safe. This happens
-     when some Mbed TLS library functions fail. Such a double-free was
-     not safe when MBEDTLS_THREADING_C was enabled on platforms where
-     freeing a mutex twice is not safe.
+   * Ensure that calling mbedtls_rsa_free() or mbedtls_entropy_free()
+     twice is safe. This happens for RSA when some Mbed TLS library functions
+     fail. Such a double-free was not safe when MBEDTLS_THREADING_C was
+     enabled on platforms where freeing a mutex twice is not safe.
    * Fix a resource leak in a bad-arguments case of mbedtls_rsa_gen_key()
      when MBEDTLS_THREADING_C is enabled on platforms where initializing
      a mutex allocates resources.

include/mbedtls/entropy.h

diff --git a/include/mbedtls/entropy.h b/include/mbedtls/entropy.h
index 5a9c11c..fa0b24f 100644
--- a/include/mbedtls/entropy.h
+++ b/include/mbedtls/entropy.h
@@ -120,13 +120,15 @@
  */
 typedef struct mbedtls_entropy_context
 {
-    int accumulator_started;
+    int accumulator_started; /* 0 after init.
+                              * 1 after the first update.
+                              * -1 after free. */
 #if defined(MBEDTLS_ENTROPY_SHA512_ACCUMULATOR)
     mbedtls_sha512_context  accumulator;
 #else
     mbedtls_sha256_context  accumulator;
 #endif
-    int             source_count;
+    int             source_count; /* Number of entries used in source. */
     mbedtls_entropy_source_state    source[MBEDTLS_ENTROPY_MAX_SOURCES];
 #if defined(MBEDTLS_HAVEGE_C)
     mbedtls_havege_state    havege_data;

library/entropy.c

diff --git a/library/entropy.c b/library/entropy.c
index db61f16..b9aca86 100644
--- a/library/entropy.c
+++ b/library/entropy.c
@@ -116,6 +116,11 @@
 
 void mbedtls_entropy_free( mbedtls_entropy_context *ctx )
 {
+    /* If the context was already free, don't call free() again.
+     * This is important for mutexes which don't allow double-free. */
+    if( ctx->accumulator_started == -1 )
+        return;
+
 #if defined(MBEDTLS_HAVEGE_C)
     mbedtls_havege_free( &ctx->havege_data );
 #endif
@@ -132,7 +137,7 @@
 #endif
     ctx->source_count = 0;
     mbedtls_platform_zeroize( ctx->source, sizeof( ctx->source ) );
-    ctx->accumulator_started = 0;
+    ctx->accumulator_started = -1;
 }
 
 int mbedtls_entropy_add_source( mbedtls_entropy_context *ctx,