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;
 }