diff options
-rw-r--r-- | crypto/property/defn_cache.c | 33 | ||||
-rw-r--r-- | crypto/property/property.c | 2 | ||||
-rw-r--r-- | crypto/property/property_local.h | 2 | ||||
-rw-r--r-- | test/property_test.c | 39 |
4 files changed, 50 insertions, 26 deletions
diff --git a/crypto/property/defn_cache.c b/crypto/property/defn_cache.c index b4cd67c990..b43f2d247e 100644 --- a/crypto/property/defn_cache.c +++ b/crypto/property/defn_cache.c @@ -82,16 +82,18 @@ OSSL_PROPERTY_LIST *ossl_prop_defn_get(OSSL_LIB_CTX *ctx, const char *prop) elem.prop = prop; r = lh_PROPERTY_DEFN_ELEM_retrieve(property_defns, &elem); ossl_lib_ctx_unlock(ctx); - return r != NULL ? r->defn : NULL; + if (r == NULL || !ossl_assert(r->defn != NULL)) + return NULL; + return r->defn; } /* - * Cache the property list for a given property string. Callers of this function - * should call ossl_prop_defn_get first to ensure that there is no existing - * cache entry for this property string. + * Cache the property list for a given property string *pl. + * If an entry already exists in the cache *pl is freed and + * overwritten with the existing entry from the cache. */ int ossl_prop_defn_set(OSSL_LIB_CTX *ctx, const char *prop, - OSSL_PROPERTY_LIST *pl) + OSSL_PROPERTY_LIST **pl) { PROPERTY_DEFN_ELEM elem, *old, *p = NULL; size_t len; @@ -109,28 +111,27 @@ int ossl_prop_defn_set(OSSL_LIB_CTX *ctx, const char *prop, if (!ossl_lib_ctx_write_lock(ctx)) return 0; + elem.prop = prop; if (pl == NULL) { - elem.prop = prop; lh_PROPERTY_DEFN_ELEM_delete(property_defns, &elem); goto end; } + /* check if property definition is in the cache already */ + if ((p = lh_PROPERTY_DEFN_ELEM_retrieve(property_defns, &elem)) != NULL) { + ossl_property_free(*pl); + *pl = p->defn; + goto end; + } len = strlen(prop); p = OPENSSL_malloc(sizeof(*p) + len); if (p != NULL) { p->prop = p->body; - p->defn = pl; + p->defn = *pl; memcpy(p->body, prop, len + 1); old = lh_PROPERTY_DEFN_ELEM_insert(property_defns, p); - if (!ossl_assert(old == NULL)) { - /* - * This should not happen. Any caller of ossl_prop_defn_set should - * have called ossl_prop_defn_get first - so we should know that - * there is no existing entry. If we get here we have a bug. We - * deliberately leak the |old| reference in order to avoid a crash - * if there are any existing users of it. - */ + if (!ossl_assert(old == NULL)) + /* This should not happen. An existing entry is handled above. */ goto end; - } if (!lh_PROPERTY_DEFN_ELEM_error(property_defns)) goto end; } diff --git a/crypto/property/property.c b/crypto/property/property.c index 2c92cb5e50..844c25cee9 100644 --- a/crypto/property/property.c +++ b/crypto/property/property.c @@ -335,7 +335,7 @@ int ossl_method_store_add(OSSL_METHOD_STORE *store, const OSSL_PROVIDER *prov, impl->properties = ossl_parse_property(store->ctx, properties); if (impl->properties == NULL) goto err; - if (!ossl_prop_defn_set(store->ctx, properties, impl->properties)) { + if (!ossl_prop_defn_set(store->ctx, properties, &impl->properties)) { ossl_property_free(impl->properties); impl->properties = NULL; goto err; diff --git a/crypto/property/property_local.h b/crypto/property/property_local.h index 6b85ce1586..797fb3bf5f 100644 --- a/crypto/property/property_local.h +++ b/crypto/property/property_local.h @@ -52,4 +52,4 @@ int ossl_property_has_optional(const OSSL_PROPERTY_LIST *query); /* Property definition cache functions */ 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); + OSSL_PROPERTY_LIST **pl); diff --git a/test/property_test.c b/test/property_test.c index 844315c226..1f4dfc3c6e 100644 --- a/test/property_test.c +++ b/test/property_test.c @@ -255,19 +255,42 @@ static int test_property_merge(int n) static int test_property_defn_cache(void) { OSSL_METHOD_STORE *store; - OSSL_PROPERTY_LIST *red, *blue; - int r = 0; + OSSL_PROPERTY_LIST *red = NULL, *blue = NULL, *blue2 = NULL; + int r; - if (TEST_ptr(store = ossl_method_store_new(NULL)) + r = TEST_ptr(store = ossl_method_store_new(NULL)) && add_property_names("red", "blue", NULL) && TEST_ptr(red = ossl_parse_property(NULL, "red")) && TEST_ptr(blue = ossl_parse_property(NULL, "blue")) && TEST_ptr_ne(red, blue) - && TEST_true(ossl_prop_defn_set(NULL, "red", red)) - && TEST_true(ossl_prop_defn_set(NULL, "blue", blue)) - && TEST_ptr_eq(ossl_prop_defn_get(NULL, "red"), red) - && TEST_ptr_eq(ossl_prop_defn_get(NULL, "blue"), blue)) - r = 1; + && TEST_true(ossl_prop_defn_set(NULL, "red", &red)); + + if (!r) { + ossl_property_free(red); + red = NULL; + ossl_property_free(blue); + blue = NULL; + } + + r = r && TEST_true(ossl_prop_defn_set(NULL, "blue", &blue)); + if (!r) { + ossl_property_free(blue); + blue = NULL; + } + + r = r && TEST_ptr_eq(ossl_prop_defn_get(NULL, "red"), red) + && TEST_ptr_eq(ossl_prop_defn_get(NULL, "blue"), blue) + && TEST_ptr(blue2 = ossl_parse_property(NULL, "blue")) + && TEST_ptr_ne(blue2, blue) + && TEST_true(ossl_prop_defn_set(NULL, "blue", &blue2)); + if (!r) { + ossl_property_free(blue2); + blue2 = NULL; + } + + r = r && TEST_ptr_eq(blue2, blue) + && TEST_ptr_eq(ossl_prop_defn_get(NULL, "blue"), blue); + ossl_method_store_free(store); return r; } |