From: Richard Levitte Date: Wed, 21 Aug 2019 07:58:10 +0000 (+0200) Subject: Modify ossl_method_store_add() to handle reference counting X-Git-Url: https://granicus.if.org/sourcecode?a=commitdiff_plain;h=b1d40ddfe23fd9551b89cdcfa52d1c23fc667f8a;p=openssl Modify ossl_method_store_add() to handle reference counting Because this function affects the reference count on failure (the call to impl_free() does this), it may as well handle incrementing it as well to indicate the extra reference in the method store. Reviewed-by: Shane Lontis (Merged from https://github.com/openssl/openssl/pull/9650) --- diff --git a/crypto/evp/evp_fetch.c b/crypto/evp/evp_fetch.c index 5c100dd1eb..41dee721a4 100644 --- a/crypto/evp/evp_fetch.c +++ b/crypto/evp/evp_fetch.c @@ -132,11 +132,9 @@ static int put_method_in_store(OPENSSL_CTX *libctx, void *store, && (store = get_default_method_store(libctx)) == NULL) return 0; - if (methdata->refcnt_up_method(method) - && ossl_method_store_add(store, methid, propdef, method, - methdata->destruct_method)) - return 1; - return 0; + return ossl_method_store_add(store, methid, propdef, method, + methdata->refcnt_up_method, + methdata->destruct_method); } static void *construct_method(const char *name, const OSSL_DISPATCH *fns, diff --git a/crypto/property/property.c b/crypto/property/property.c index c3fa8df9c6..6576ba0fd2 100644 --- a/crypto/property/property.c +++ b/crypto/property/property.c @@ -174,8 +174,9 @@ static int ossl_method_store_insert(OSSL_METHOD_STORE *store, ALGORITHM *alg) } int ossl_method_store_add(OSSL_METHOD_STORE *store, - int nid, const char *properties, - void *method, void (*method_destruct)(void *)) + int nid, const char *properties, void *method, + int (*method_up_ref)(void *), + void (*method_destruct)(void *)) { ALGORITHM *alg = NULL; IMPLEMENTATION *impl; @@ -190,6 +191,8 @@ int ossl_method_store_add(OSSL_METHOD_STORE *store, impl = OPENSSL_malloc(sizeof(*impl)); if (impl == NULL) return 0; + if (method_up_ref != NULL && !method_up_ref(method)) + return 0; impl->method = method; impl->method_destruct = method_destruct; diff --git a/doc/internal/man3/OSSL_METHOD_STORE.pod b/doc/internal/man3/OSSL_METHOD_STORE.pod index abe7ebe317..be439f15b5 100644 --- a/doc/internal/man3/OSSL_METHOD_STORE.pod +++ b/doc/internal/man3/OSSL_METHOD_STORE.pod @@ -20,8 +20,9 @@ ossl_method_store_cache_get, ossl_method_store_cache_set int ossl_method_store_init(OPENSSL_CTX *ctx); void ossl_method_store_cleanup(OPENSSL_CTX *ctx); int ossl_method_store_add(OSSL_METHOD_STORE *store, - int nid, const char *properties, - void *method, void (*method_destruct)(void *)); + int nid, const char *properties, void *method, + int (*method_up_ref)(void *), + void (*method_destruct)(void *)); int ossl_method_store_remove(OSSL_METHOD_STORE *store, int nid, const void *method); int ossl_method_store_fetch(OSSL_METHOD_STORE *store, @@ -64,8 +65,11 @@ ossl_method_store_free() frees resources allocated to B. ossl_method_store_add() adds the B to the B as an instance of an algorithm indicated by B and the property definition B. -The optional B function is called when B is being -released from B. +If the B function is given, it's called to increment the +reference count of the method. +If the B function is given, it's called when this function +fails to add the method to the store, or later on when it is being released from +the B. ossl_method_store_remove() removes the B identified by B from the B. diff --git a/include/internal/property.h b/include/internal/property.h index a916be3cc5..e7e7f574e8 100644 --- a/include/internal/property.h +++ b/include/internal/property.h @@ -20,6 +20,7 @@ OSSL_METHOD_STORE *ossl_method_store_new(OPENSSL_CTX *ctx); void ossl_method_store_free(OSSL_METHOD_STORE *store); int ossl_method_store_add(OSSL_METHOD_STORE *store, int nid, const char *properties, void *implementation, + int (*implementation_up_ref)(void *), void (*implementation_destruct)(void *)); int ossl_method_store_remove(OSSL_METHOD_STORE *store, int nid, const void *implementation); diff --git a/test/property_test.c b/test/property_test.c index 765416a11d..a47dcfc09b 100644 --- a/test/property_test.c +++ b/test/property_test.c @@ -241,7 +241,7 @@ static int test_register_deregister(void) for (i = 0; i < OSSL_NELEM(impls); i++) if (!TEST_true(ossl_method_store_add(store, impls[i].nid, impls[i].prop, - impls[i].impl, NULL))) { + impls[i].impl, NULL, NULL))) { TEST_note("iteration %zd", i + 1); goto err; } @@ -308,7 +308,7 @@ static int test_property(void) for (i = 0; i < OSSL_NELEM(impls); i++) if (!TEST_true(ossl_method_store_add(store, impls[i].nid, impls[i].prop, - impls[i].impl, NULL))) { + impls[i].impl, NULL, NULL))) { TEST_note("iteration %zd", i + 1); goto err; } @@ -347,7 +347,7 @@ static int test_query_cache_stochastic(void) for (i = 1; i <= max; i++) { v[i] = 2 * i; BIO_snprintf(buf, sizeof(buf), "n=%d\n", i); - if (!TEST_true(ossl_method_store_add(store, i, buf, "abc", NULL)) + if (!TEST_true(ossl_method_store_add(store, i, buf, "abc", NULL, NULL)) || !TEST_true(ossl_method_store_cache_set(store, i, buf, v + i)) || !TEST_true(ossl_method_store_cache_set(store, i, "n=1234", "miss"))) {