[openssl] master update

Dr. Paul Dale pauli at openssl.org
Thu Jun 24 05:52:51 UTC 2021


The branch master has been updated
       via  2fee3a77f8179c8e4c0e33d622549270b380fa8a (commit)
       via  3377f34fb8bcf560c83627ed44c99c09c0b6a772 (commit)
       via  e3c507797acc6049f5791062cd9b2fa5fe0aa824 (commit)
      from  0218bcdd3feab456135207c140998305df73ab7b (commit)


- Log -----------------------------------------------------------------
commit 2fee3a77f8179c8e4c0e33d622549270b380fa8a
Author: Pauli <pauli at openssl.org>
Date:   Wed Jun 23 14:18:25 2021 +1000

    property: remove spurious incorrect comments
    
    Reviewed-by: Tomas Mraz <tomas at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/15871)

commit 3377f34fb8bcf560c83627ed44c99c09c0b6a772
Author: Pauli <pauli at openssl.org>
Date:   Wed Jun 23 14:18:07 2021 +1000

    property: add locking for the property string database
    
    This previously relied on the caller locking the property store correctly.
    This is no longer the case so the string database now requires locking.
    
    Fixes #15866
    
    Reviewed-by: Tomas Mraz <tomas at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/15871)

commit e3c507797acc6049f5791062cd9b2fa5fe0aa824
Author: Pauli <pauli at openssl.org>
Date:   Wed Jun 23 14:17:59 2021 +1000

    err: add unable to get lock errors
    
    Reviewed-by: Tomas Mraz <tomas at openssl.org>
    (Merged from https://github.com/openssl/openssl/pull/15871)

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

Summary of changes:
 crypto/err/err.c                  |  2 ++
 crypto/property/property.c        | 12 ++------
 crypto/property/property_string.c | 61 +++++++++++++++++++++++++++------------
 include/openssl/err.h.in          |  2 ++
 4 files changed, 49 insertions(+), 28 deletions(-)

diff --git a/crypto/err/err.c b/crypto/err/err.c
index 9b1a15d5bd..60a9b02d19 100644
--- a/crypto/err/err.c
+++ b/crypto/err/err.c
@@ -127,6 +127,8 @@ static ERR_STRING_DATA ERR_str_reasons[] = {
     {ERR_R_FETCH_FAILED, "fetch failed"},
 
     {ERR_R_INVALID_PROPERTY_DEFINITION, "invalid property definition"},
+    {ERR_R_UNABLE_TO_GET_READ_LOCK, "unable to get read lock"},
+    {ERR_R_UNABLE_TO_GET_WRITE_LOCK, "unable to get write lock"},
     {0, NULL},
 };
 #endif
diff --git a/crypto/property/property.c b/crypto/property/property.c
index c3f1c5ac58..a4cd612b9d 100644
--- a/crypto/property/property.c
+++ b/crypto/property/property.c
@@ -273,12 +273,7 @@ int ossl_method_store_add(OSSL_METHOD_STORE *store, const OSSL_PROVIDER *prov,
     }
     impl->provider = prov;
 
-    /*
-     * Insert into the hash table if required.
-     *
-     * A write lock is used unconditionally because we wend our way down to the
-     * property string code which isn't locking friendly.
-     */
+    /* Insert into the hash table if required */
     if (!ossl_property_write_lock(store)) {
         OPENSSL_free(impl);
         return 0;
@@ -418,10 +413,7 @@ int ossl_method_store_fetch(OSSL_METHOD_STORE *store, int nid,
     if (nid <= 0 || method == NULL || store == NULL)
         return 0;
 
-    /*
-     * This only needs to be a read lock, because queries never create property
-     * names or value and thus don't modify any of the property string layer.
-     */
+    /* This only needs to be a read lock, because the query won't create anything */
     if (!ossl_property_read_lock(store))
         return 0;
     alg = ossl_method_store_retrieve(store, nid);
diff --git a/crypto/property/property_string.c b/crypto/property/property_string.c
index c9fde70a76..38deab5af0 100644
--- a/crypto/property/property_string.c
+++ b/crypto/property/property_string.c
@@ -35,6 +35,7 @@ DEFINE_LHASH_OF(PROPERTY_STRING);
 typedef LHASH_OF(PROPERTY_STRING) PROP_TABLE;
 
 typedef struct {
+    CRYPTO_RWLOCK *lock;
     PROP_TABLE *prop_names;
     PROP_TABLE *prop_values;
     OSSL_PROPERTY_IDX prop_name_idx;
@@ -74,6 +75,7 @@ static void property_string_data_free(void *vpropdata)
     if (propdata == NULL)
         return;
 
+    CRYPTO_THREAD_lock_free(propdata->lock);
     property_table_free(&propdata->prop_names);
     property_table_free(&propdata->prop_values);
     propdata->prop_name_idx = propdata->prop_value_idx = 0;
@@ -87,6 +89,10 @@ static void *property_string_data_new(OSSL_LIB_CTX *ctx) {
     if (propdata == NULL)
         return NULL;
 
+    propdata->lock = CRYPTO_THREAD_lock_new();
+    if (propdata->lock == NULL)
+        goto err;
+
     propdata->prop_names = lh_PROPERTY_STRING_new(&property_hash,
                                                   &property_cmp);
     if (propdata->prop_names == NULL)
@@ -128,40 +134,40 @@ static PROPERTY_STRING *new_property_string(const char *s,
     return ps;
 }
 
-static OSSL_PROPERTY_IDX ossl_property_string(PROP_TABLE *t,
+static OSSL_PROPERTY_IDX ossl_property_string(CRYPTO_RWLOCK *lock,
+                                              PROP_TABLE *t,
                                               OSSL_PROPERTY_IDX *pidx,
                                               const char *s)
 {
     PROPERTY_STRING p, *ps, *ps_new;
 
     p.s = s;
+    if (!CRYPTO_THREAD_read_lock(lock)) {
+        ERR_raise(ERR_LIB_CRYPTO, ERR_R_UNABLE_TO_GET_READ_LOCK);
+        return 0;
+    }
     ps = lh_PROPERTY_STRING_retrieve(t, &p);
-    if (ps == NULL && pidx != NULL)
-        if ((ps_new = new_property_string(s, pidx)) != NULL) {
+    if (ps == NULL && pidx != NULL) {
+        CRYPTO_THREAD_unlock(lock);
+        if (!CRYPTO_THREAD_write_lock(lock)) {
+            ERR_raise(ERR_LIB_CRYPTO, ERR_R_UNABLE_TO_GET_WRITE_LOCK);
+            return 0;
+        }
+        ps = lh_PROPERTY_STRING_retrieve(t, &p);
+        if (ps == NULL && (ps_new = new_property_string(s, pidx)) != NULL) {
             lh_PROPERTY_STRING_insert(t, ps_new);
             if (lh_PROPERTY_STRING_error(t)) {
                 property_free(ps_new);
+                CRYPTO_THREAD_unlock(lock);
                 return 0;
             }
             ps = ps_new;
         }
+    }
+    CRYPTO_THREAD_unlock(lock);
     return ps != NULL ? ps->idx : 0;
 }
 
-OSSL_PROPERTY_IDX ossl_property_name(OSSL_LIB_CTX *ctx, const char *s,
-                                     int create)
-{
-    PROPERTY_STRING_DATA *propdata
-        = ossl_lib_ctx_get_data(ctx, OSSL_LIB_CTX_PROPERTY_STRING_INDEX,
-                                &property_string_data_method);
-
-    if (propdata == NULL)
-        return 0;
-    return ossl_property_string(propdata->prop_names,
-                                create ? &propdata->prop_name_idx : NULL,
-                                s);
-}
-
 struct find_str_st {
     const char *str;
     OSSL_PROPERTY_IDX idx;
@@ -189,13 +195,32 @@ static const char *ossl_property_str(int name, OSSL_LIB_CTX *ctx,
     findstr.str = NULL;
     findstr.idx = idx;
 
+    if (!CRYPTO_THREAD_read_lock(propdata->lock)) {
+        ERR_raise(ERR_LIB_CRYPTO, ERR_R_UNABLE_TO_GET_READ_LOCK);
+        return NULL;
+    }
     lh_PROPERTY_STRING_doall_arg(name ? propdata->prop_names
                                       : propdata->prop_values,
                                  find_str_fn, &findstr);
+    CRYPTO_THREAD_unlock(propdata->lock);
 
     return findstr.str;
 }
 
+OSSL_PROPERTY_IDX ossl_property_name(OSSL_LIB_CTX *ctx, const char *s,
+                                     int create)
+{
+    PROPERTY_STRING_DATA *propdata
+        = ossl_lib_ctx_get_data(ctx, OSSL_LIB_CTX_PROPERTY_STRING_INDEX,
+                                &property_string_data_method);
+
+    if (propdata == NULL)
+        return 0;
+    return ossl_property_string(propdata->lock, propdata->prop_names,
+                                create ? &propdata->prop_name_idx : NULL,
+                                s);
+}
+
 const char *ossl_property_name_str(OSSL_LIB_CTX *ctx, OSSL_PROPERTY_IDX idx)
 {
     return ossl_property_str(1, ctx, idx);
@@ -210,7 +235,7 @@ OSSL_PROPERTY_IDX ossl_property_value(OSSL_LIB_CTX *ctx, const char *s,
 
     if (propdata == NULL)
         return 0;
-    return ossl_property_string(propdata->prop_values,
+    return ossl_property_string(propdata->lock, propdata->prop_values,
                                 create ? &propdata->prop_value_idx : NULL,
                                 s);
 }
diff --git a/include/openssl/err.h.in b/include/openssl/err.h.in
index 4bc5d1eea5..306656a2c1 100644
--- a/include/openssl/err.h.in
+++ b/include/openssl/err.h.in
@@ -358,6 +358,8 @@ static ossl_unused ossl_inline int ERR_COMMON_ERROR(unsigned long errcode)
 # define ERR_R_UNSUPPORTED                       (268|ERR_RFLAG_COMMON)
 # define ERR_R_FETCH_FAILED                      (269|ERR_RFLAG_COMMON)
 # define ERR_R_INVALID_PROPERTY_DEFINITION       (270|ERR_RFLAG_COMMON)
+# define ERR_R_UNABLE_TO_GET_READ_LOCK           (271|ERR_R_FATAL)
+# define ERR_R_UNABLE_TO_GET_WRITE_LOCK          (272|ERR_R_FATAL)
 
 typedef struct ERR_string_data_st {
     unsigned long error;


More information about the openssl-commits mailing list