Fix data race setting `default_DSO_meth`
authorPauli <pauli@openssl.org>
Fri, 5 Nov 2021 03:10:10 +0000 (13:10 +1000)
committerPauli <pauli@openssl.org>
Sun, 7 Nov 2021 22:58:38 +0000 (08:58 +1000)
The global variable `default_DSO_meth` was potentially set multiple times by
different threads.  It turns out that it could only be set to a single value
so the race is harmless but still better avoided.  The fix here simply removes
the global and accesses the value it was set to via the `DSO_METHOD_openssl()`
call.

Problem discovered via #16970, but this does not resolve that issue because
there are other concerns.

Reviewed-by: Tomas Mraz <tomas@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/16972)

crypto/dso/dso_lib.c

index 4850e96a4b5e9483bf0421e8b142fb8d60f9ab48..e093b77a272dd81441700f8f94a9bef36a5119d9 100644 (file)
 #include "dso_local.h"
 #include "internal/refcount.h"
 
-static DSO_METHOD *default_DSO_meth = NULL;
-
 static DSO *DSO_new_method(DSO_METHOD *meth)
 {
     DSO *ret;
 
-    if (default_DSO_meth == NULL) {
-        /*
-         * We default to DSO_METH_openssl() which in turn defaults to
-         * stealing the "best available" method. Will fallback to
-         * DSO_METH_null() in the worst case.
-         */
-        default_DSO_meth = DSO_METHOD_openssl();
-    }
     ret = OPENSSL_zalloc(sizeof(*ret));
     if (ret == NULL) {
         ERR_raise(ERR_LIB_DSO, ERR_R_MALLOC_FAILURE);
@@ -36,7 +26,7 @@ static DSO *DSO_new_method(DSO_METHOD *meth)
         OPENSSL_free(ret);
         return NULL;
     }
-    ret->meth = default_DSO_meth;
+    ret->meth = DSO_METHOD_openssl();
     ret->references = 1;
     ret->lock = CRYPTO_THREAD_lock_new();
     if (ret->lock == NULL) {
@@ -309,9 +299,8 @@ char *DSO_convert_filename(DSO *dso, const char *filename)
 
 int DSO_pathbyaddr(void *addr, char *path, int sz)
 {
-    DSO_METHOD *meth = default_DSO_meth;
-    if (meth == NULL)
-        meth = DSO_METHOD_openssl();
+    DSO_METHOD *meth = DSO_METHOD_openssl();
+
     if (meth->pathbyaddr == NULL) {
         ERR_raise(ERR_LIB_DSO, DSO_R_UNSUPPORTED);
         return -1;
@@ -339,9 +328,8 @@ DSO *DSO_dsobyaddr(void *addr, int flags)
 
 void *DSO_global_lookup(const char *name)
 {
-    DSO_METHOD *meth = default_DSO_meth;
-    if (meth == NULL)
-        meth = DSO_METHOD_openssl();
+    DSO_METHOD *meth = DSO_METHOD_openssl();
+
     if (meth->globallookup == NULL) {
         ERR_raise(ERR_LIB_DSO, DSO_R_UNSUPPORTED);
         return NULL;