property: check return values from the property locking calls.

A failure to obtain a lock would have resulted in much badness, now it results
in a failure return.

Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/14773)

crypto/evp/evp_fetch.c

@@ -371,12 +371,13 @@ void *evp_generic_fetch_by_number(OSSL_LIB_CTX *libctx, int operation_id,
                                    free_method);
 }
 
-void evp_method_store_flush(OSSL_LIB_CTX *libctx)
+int evp_method_store_flush(OSSL_LIB_CTX *libctx)
 {
     OSSL_METHOD_STORE *store = get_evp_method_store(libctx);
 
     if (store != NULL)
-        ossl_method_store_flush_cache(store, 1);
+        return ossl_method_store_flush_cache(store, 1);
+    return 1;
 }
 
 static int evp_set_parsed_default_properties(OSSL_LIB_CTX *libctx,
@@ -390,7 +391,7 @@ static int evp_set_parsed_default_properties(OSSL_LIB_CTX *libctx,
         ossl_property_free(*plp);
         *plp = def_prop;
         if (store != NULL)
-            ossl_method_store_flush_cache(store, 0);
+            return ossl_method_store_flush_cache(store, 0);
         return 1;
     }
     ERR_raise(ERR_LIB_EVP, ERR_R_INTERNAL_ERROR);

crypto/property/property.c

@@ -117,17 +117,17 @@ static void ossl_method_free(METHOD *method)
     (*method->free)(method->method);
 }
 
-int ossl_property_read_lock(OSSL_METHOD_STORE *p)
+static __owur int ossl_property_read_lock(OSSL_METHOD_STORE *p)
 {
     return p != NULL ? CRYPTO_THREAD_read_lock(p->lock) : 0;
 }
 
-int ossl_property_write_lock(OSSL_METHOD_STORE *p)
+static __owur int ossl_property_write_lock(OSSL_METHOD_STORE *p)
 {
     return p != NULL ? CRYPTO_THREAD_write_lock(p->lock) : 0;
 }
 
-int ossl_property_unlock(OSSL_METHOD_STORE *p)
+static int ossl_property_unlock(OSSL_METHOD_STORE *p)
 {
     return p != 0 ? CRYPTO_THREAD_unlock(p->lock) : 0;
 }
@@ -246,7 +246,10 @@ int ossl_method_store_add(OSSL_METHOD_STORE *store, const OSSL_PROVIDER *prov,
      * A write lock is used unconditionally because we wend our way down to the
      * property string code which isn't locking friendly.
      */
-    ossl_property_write_lock(store);
+    if (!ossl_property_write_lock(store)) {
+        OPENSSL_free(impl);
+        return 0;
+    }
     ossl_method_cache_flush(store, nid);
     if ((impl->properties = ossl_prop_defn_get(store->ctx, properties)) == NULL) {
         impl->properties = ossl_parse_property(store->ctx, properties);
@@ -298,7 +301,8 @@ int ossl_method_store_remove(OSSL_METHOD_STORE *store, int nid,
     if (nid <= 0 || method == NULL || store == NULL)
         return 0;
 
-    ossl_property_write_lock(store);
+    if (!ossl_property_write_lock(store))
+        return 0;
     ossl_method_cache_flush(store, nid);
     alg = ossl_method_store_retrieve(store, nid);
     if (alg == NULL) {
@@ -349,7 +353,8 @@ int ossl_method_store_fetch(OSSL_METHOD_STORE *store, int nid,
      * 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.
      */
-    ossl_property_read_lock(store);
+    if (!ossl_property_read_lock(store))
+        return 0;
     alg = ossl_method_store_retrieve(store, nid);
     if (alg == NULL) {
         ossl_property_unlock(store);
@@ -425,14 +430,16 @@ static void ossl_method_cache_flush(OSSL_METHOD_STORE *store, int nid)
     }
 }
 
-void ossl_method_store_flush_cache(OSSL_METHOD_STORE *store, int all)
+int ossl_method_store_flush_cache(OSSL_METHOD_STORE *store, int all)
 {
     void *arg = (all != 0 ? store->algs : NULL);
 
-    ossl_property_write_lock(store);
+    if (!ossl_property_write_lock(store))
+        return 0;
     ossl_sa_ALGORITHM_doall_arg(store->algs, &impl_cache_flush_alg, arg);
     store->nelem = 0;
     ossl_property_unlock(store);
+    return 1;
 }
 
 IMPLEMENT_LHASH_DOALL_ARG(QUERY, IMPL_CACHE_FLUSH);
@@ -508,7 +515,8 @@ int ossl_method_store_cache_get(OSSL_METHOD_STORE *store, int nid,
     if (nid <= 0 || store == NULL)
         return 0;
 
-    ossl_property_read_lock(store);
+    if (!ossl_property_read_lock(store))
+        return 0;
     alg = ossl_method_store_retrieve(store, nid);
     if (alg == NULL)
         goto err;
@@ -541,7 +549,8 @@ int ossl_method_store_cache_set(OSSL_METHOD_STORE *store, int nid,
     if (prop_query == NULL)
         return 1;
 
-    ossl_property_write_lock(store);
+    if (!ossl_property_write_lock(store))
+        return 0;
     if (store->need_flush)
         ossl_method_cache_flush_some(store);
     alg = ossl_method_store_retrieve(store, nid);

crypto/property/property_local.h

@@ -27,9 +27,3 @@ int ossl_property_has_optional(const OSSL_PROPERTY_LIST *query);
 OSSL_PROPERTY_LIST *ossl_prop_defn_get(OSSL_LIB_CTX *ctx, const char *prop);
 int ossl_prop_defn_set(OSSL_LIB_CTX *ctx, const char *prop,
                        OSSL_PROPERTY_LIST *pl);
-
-/* Property cache lock / unlock */
-int ossl_property_write_lock(OSSL_METHOD_STORE *);
-int ossl_property_read_lock(OSSL_METHOD_STORE *);
-int ossl_property_unlock(OSSL_METHOD_STORE *);
-

crypto/provider_core.c

@@ -956,7 +956,7 @@ int ossl_provider_self_test(const OSSL_PROVIDER *prov)
         return 1;
     ret = prov->self_test(prov->provctx);
     if (ret == 0)
-        evp_method_store_flush(ossl_provider_libctx(prov));
+        (void)evp_method_store_flush(ossl_provider_libctx(prov));
     return ret;
 }
 

doc/internal/man3/OSSL_METHOD_STORE.pod

@@ -104,8 +104,8 @@ ossl_method_store_new() returns a new method store object or NULL on failure.
 
 ossl_method_store_free(), ossl_method_store_add(),
 ossl_method_store_remove(), ossl_method_store_fetch(),
-ossl_method_store_cache_get()
+ossl_method_store_cache_get(), ossl_method_store_cache_set() and
-and ossl_method_store_cache_set() return B<1> on success and B<0> on error.
+ossl_method_store_flush_cache() return B<1> on success and B<0> on error.
 
 ossl_method_store_free() and ossl_method_store_cleanup() do not return any value.
 

include/crypto/evp.h

@@ -868,7 +868,7 @@ int evp_pkey_ctx_get1_id_len_prov(EVP_PKEY_CTX *ctx, size_t *id_len);
 int evp_pkey_ctx_use_cached_data(EVP_PKEY_CTX *ctx);
 # endif /* !defined(FIPS_MODULE) */
 
-void evp_method_store_flush(OSSL_LIB_CTX *libctx);
+int evp_method_store_flush(OSSL_LIB_CTX *libctx);
 int evp_set_default_properties_int(OSSL_LIB_CTX *libctx, const char *propq,
                                    int loadconfig);
 

include/internal/property.h

@@ -58,7 +58,7 @@ int ossl_method_store_cache_set(OSSL_METHOD_STORE *store, int nid,
                                 int (*method_up_ref)(void *),
                                 void (*method_destruct)(void *));
 
-void ossl_method_store_flush_cache(OSSL_METHOD_STORE *store, int all);
+__owur int ossl_method_store_flush_cache(OSSL_METHOD_STORE *store, int all);
 
 /* Merge two property queries together */
 OSSL_PROPERTY_LIST *ossl_property_merge(const OSSL_PROPERTY_LIST *a,