[openssl-commits] [openssl] master update

Richard Levitte levitte at openssl.org
Tue Dec 12 16:26:12 UTC 2017


The branch master has been updated
       via  aef84bb4efbddfd95d042f3f5f1d362ed7d4faeb (commit)
      from  ea7df7ea449ef85a163fde917906e6e3da9388e5 (commit)


- Log -----------------------------------------------------------------
commit aef84bb4efbddfd95d042f3f5f1d362ed7d4faeb
Author: Richard Levitte <levitte at openssl.org>
Date:   Tue Dec 12 02:05:38 2017 +0100

    Fix leak in ERR_get_state() when OPENSSL_init_crypto() isn't called yet
    
    If OPENSSL_init_crypto() hasn't been called yet when ERR_get_state()
    is called, it need to be called early, so the base initialization is
    done.  On some platforms (those who support DSO functionality and
    don't define OPENSSL_USE_NODELETE), that includes a call of
    ERR_set_mark(), which calls this function again.
    Furthermore, we know that ossl_init_thread_start(), which is called
    later in ERR_get_state(), calls OPENSSL_init_crypto(0, NULL), except
    that's too late.
    Here's what happens without an early call of OPENSSL_init_crypto():
    
        => ERR_get_state():
             => CRYPTO_THREAD_get_local():
             <= NULL;
             # no state is found, so it gets allocated.
             => ossl_init_thread_start():
                  => OPENSSL_init_crypto():
                       # Here, base_inited is set to 1
                       # before ERR_set_mark() call
                       => ERR_set_mark():
                            => ERR_get_state():
                                 => CRYPTO_THREAD_get_local():
                                 <= NULL;
                                 # no state is found, so it gets allocated!!!!!
                                 => ossl_init_thread_start():
                                      => OPENSSL_init_crypto():
                                           # base_inited is 1,
                                           # so no more init to be done
                                      <= 1
                                 <=
                                 => CRYPTO_thread_set_local():
                                 <=
                            <=
                       <=
                  <= 1
             <=
             => CRYPTO_thread_set_local()      # previous value removed!
        <=
    
    Result: double allocation, and we have a leak.
    
    By calling the base OPENSSL_init_crypto() early, we get this instead:
    
        => ERR_get_state():
             => OPENSSL_init_crypto():
                  # Here, base_inited is set to 1
                  # before ERR_set_mark() call
                  => ERR_set_mark():
                       => ERR_get_state():
                            => OPENSSL_init_crypto():
                                 # base_inited is 1,
                                 # so no more init to be done
                            <= 1
                            => CRYPTO_THREAD_get_local():
                            <= NULL;
                            # no state is found, so it gets allocated
                            # let's assume we got 0xDEADBEEF
                            => ossl_init_thread_start():
                                 => OPENSSL_init_crypto():
                                      # base_inited is 1,
                                      # so no more init to be done
                                 <= 1
                            <= 1
                            => CRYPTO_thread_set_local():
                            <=
                       <=
                  <=
             <= 1
             => CRYPTO_THREAD_get_local():
             <= 0xDEADBEEF
        <= 0xDEADBEEF
    
    Result: no leak.
    
    Reviewed-by: Rich Salz <rsalz at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/4913)

-----------------------------------------------------------------------

Summary of changes:
 crypto/err/err.c | 8 ++++++++
 1 file changed, 8 insertions(+)

diff --git a/crypto/err/err.c b/crypto/err/err.c
index bd9e062..c0bdbb8 100644
--- a/crypto/err/err.c
+++ b/crypto/err/err.c
@@ -667,6 +667,14 @@ ERR_STATE *ERR_get_state(void)
     if (!RUN_ONCE(&err_init, err_do_init))
         return NULL;
 
+    /*
+     * If base OPENSSL_init_crypto() hasn't been called yet, be sure to call
+     * it now to avoid state to be doubly allocated and thereby leak memory.
+     * Needed on any platform that doesn't define OPENSSL_USE_NODELETE.
+     */
+    if (!OPENSSL_init_crypto(0, NULL))
+        return NULL;
+
     state = CRYPTO_THREAD_get_local(&err_thread_local);
 
     if (state == NULL) {


More information about the openssl-commits mailing list