mirror/openssl
2
0
Fork 0
mirror of https://github.com/openssl/openssl.git synced 2025-02-17 14:32:04 +08:00
Code Issues Packages Projects Releases Wiki Activity

Properties: make query cache reference count aware.

Browse Source 
The property query cache was not reference count aware and this could cause
problems if the property store removes an algorithm while it is being returned
from an asynchronous query.  This change makes the cache reference count aware
and avoids disappearing algorithms.

A side effect of this change is that the reference counts are now owned by the
cache and store.

Reviewed-by: Richard Levitte <levitte@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/10408)
This commit is contained in:
Pauli 2019-11-11 11:17:32 +10:00
parent f75abcc0f0
commit bdbf2df2e6
5 changed files with 104 additions and 60 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

16
crypto/evp/evp_fetch.c
View File

@ -186,13 +186,9 @@ static void *get_evp_method_from_store(OPENSSL_CTX *libctx, void *store,
&& (store = get_evp_method_store(libctx)) == NULL)
return NULL;
(void)ossl_method_store_fetch(store, meth_id, methdata->propquery,
&method);
if (method != NULL
&& !methdata->refcnt_up_method(method)) {
method = NULL;
}
if (!ossl_method_store_fetch(store, meth_id, methdata->propquery,
&method))
return NULL;
return method;
}
@ -328,7 +324,6 @@ inner_evp_generic_fetch(OPENSSL_CTX *libctx, int operation_id,
mcmdata.names = name;
mcmdata.propquery = properties;
mcmdata.method_from_dispatch = new_method;
mcmdata.destruct_method = free_method;
mcmdata.refcnt_up_method = up_ref_method;
mcmdata.destruct_method = free_method;
if ((method = ossl_method_construct(libctx, operation_id,
@ -343,10 +338,9 @@ inner_evp_generic_fetch(OPENSSL_CTX *libctx, int operation_id,
if (name_id == 0)
name_id = ossl_namemap_name2num(namemap, name);
meth_id = evp_method_id(operation_id, name_id);
ossl_method_store_cache_set(store, meth_id, properties, method);
ossl_method_store_cache_set(store, meth_id, properties, method,
up_ref_method, free_method);
}
} else {
up_ref_method(method);
}
return method;

116
crypto/property/property.c
View File

@ -22,20 +22,25 @@
#include "property_local.h"
/* The number of elements in the query cache before we initiate a flush */
#define IMPL_CACHE_FLUSH_THRESHOLD 500
#define IMPL_CACHE_FLUSH_THRESHOLD 50
typedef struct {
void *method;
int (*up_ref)(void *);
void (*free)(void *);
} METHOD;
typedef struct {
const OSSL_PROVIDER *provider;
OSSL_PROPERTY_LIST *properties;
void *method;
void (*method_destruct)(void *);
METHOD method;
} IMPLEMENTATION;
DEFINE_STACK_OF(IMPLEMENTATION)
typedef struct {
const char *query;
void *method;
METHOD method;
char body[1];
} QUERY;
@ -67,6 +72,16 @@ DEFINE_SPARSE_ARRAY_OF(ALGORITHM);
static void ossl_method_cache_flush(OSSL_METHOD_STORE *store, int nid);
static void ossl_method_cache_flush_all(OSSL_METHOD_STORE *c);
static int ossl_method_up_ref(METHOD *method)
{
return (*method->up_ref)(method->method);
}
static void ossl_method_free(METHOD *method)
{
(*method->free)(method->method);
}
int ossl_property_read_lock(OSSL_METHOD_STORE *p)
{
return p != NULL ? CRYPTO_THREAD_read_lock(p->lock) : 0;
@ -95,15 +110,17 @@ static int query_cmp(const QUERY *a, const QUERY *b)
static void impl_free(IMPLEMENTATION *impl)
{
if (impl != NULL) {
if (impl->method_destruct)
impl->method_destruct(impl->method);
ossl_method_free(&impl->method);
OPENSSL_free(impl);
}
}
static void impl_cache_free(QUERY *elem)
{
OPENSSL_free(elem);
if (elem != NULL) {
ossl_method_free(&elem->method);
OPENSSL_free(elem);
}
}
static void alg_cleanup(ossl_uintmax_t idx, ALGORITHM *a)
@ -132,7 +149,7 @@ OSSL_METHOD_STORE *ossl_method_store_new(OPENSSL_CTX *ctx)
return NULL;
}
if ((res->lock = CRYPTO_THREAD_lock_new()) == NULL) {
OPENSSL_free(res->algs);
ossl_sa_ALGORITHM_free(res->algs);
OPENSSL_free(res);
return NULL;
}
@ -180,13 +197,14 @@ int ossl_method_store_add(OSSL_METHOD_STORE *store, const OSSL_PROVIDER *prov,
impl = OPENSSL_malloc(sizeof(*impl));
if (impl == NULL)
return 0;
if (method_up_ref != NULL && !method_up_ref(method)) {
impl->method.method = method;
impl->method.up_ref = method_up_ref;
impl->method.free = method_destruct;
if (!ossl_method_up_ref(&impl->method)) {
OPENSSL_free(impl);
return 0;
}
impl->provider = prov;
impl->method = method;
impl->method_destruct = method_destruct;
/*
* Insert into the hash table if required.
@ -262,10 +280,10 @@ int ossl_method_store_remove(OSSL_METHOD_STORE *store, int nid,
for (i = 0; i < sk_IMPLEMENTATION_num(alg->impls); i++) {
IMPLEMENTATION *impl = sk_IMPLEMENTATION_value(alg->impls, i);
if (impl->method == method) {
if (impl->method.method == method) {
impl_free(impl);
sk_IMPLEMENTATION_delete(alg->impls, i);
ossl_property_unlock(store);
impl_free(impl);
return 1;
}
}
@ -279,6 +297,7 @@ int ossl_method_store_fetch(OSSL_METHOD_STORE *store, int nid,
ALGORITHM *alg;
IMPLEMENTATION *impl;
OSSL_PROPERTY_LIST *pq = NULL, *p2;
METHOD *best_method = NULL;
int ret = 0;
int j, best = -1, score, optional;
@ -302,7 +321,7 @@ int ossl_method_store_fetch(OSSL_METHOD_STORE *store, int nid,
if (prop_query == NULL) {
if ((impl = sk_IMPLEMENTATION_value(alg->impls, 0)) != NULL) {
*method = impl->method;
best_method = &impl->method;
ret = 1;
}
goto fin;
@ -322,14 +341,18 @@ int ossl_method_store_fetch(OSSL_METHOD_STORE *store, int nid,
impl = sk_IMPLEMENTATION_value(alg->impls, j);
score = ossl_property_match_count(pq, impl->properties);
if (score > best) {
*method = impl->method;
best_method = &impl->method;
best = score;
ret = 1;
if (!optional)
goto fin;
best = score;
}
}
fin:
if (ret && ossl_method_up_ref(best_method))
*method = best_method->method;
else
ret = 0;
ossl_property_unlock(store);
ossl_property_free(pq);
return ret;
@ -414,7 +437,7 @@ static void impl_cache_flush_cache(QUERY *c, IMPL_CACHE_FLUSH *state)
state->seed = n;
if ((n & 1) != 0)
OPENSSL_free(lh_QUERY_delete(state->cache, c));
impl_cache_free(lh_QUERY_delete(state->cache, c));
else
state->nelem++;
}
@ -446,34 +469,38 @@ int ossl_method_store_cache_get(OSSL_METHOD_STORE *store, int nid,
{
ALGORITHM *alg;
QUERY elem, *r;
int res = 0;
if (nid <= 0 || store == NULL)
return 0;
ossl_property_read_lock(store);
alg = ossl_method_store_retrieve(store, nid);
if (alg == NULL) {
ossl_property_unlock(store);
return 0;
}
if (alg == NULL)
goto err;
elem.query = prop_query != NULL ? prop_query : "";
r = lh_QUERY_retrieve(alg->cache, &elem);
if (r == NULL) {
ossl_property_unlock(store);
return 0;
if (r == NULL)
goto err;
if (ossl_method_up_ref(&r->method)) {
*method = r->method.method;
res = 1;
}
*method = r->method;
err:
ossl_property_unlock(store);
return 1;
return res;
}
int ossl_method_store_cache_set(OSSL_METHOD_STORE *store, int nid,
const char *prop_query, void *method)
const char *prop_query, void *method,
int (*method_up_ref)(void *),
void (*method_destruct)(void *))
{
QUERY elem, *old, *p = NULL;
ALGORITHM *alg;
size_t len;
int res = 1;
if (nid <= 0 || store == NULL)
return 0;
@ -484,36 +511,41 @@ int ossl_method_store_cache_set(OSSL_METHOD_STORE *store, int nid,
if (store->need_flush)
ossl_method_cache_flush_some(store);
alg = ossl_method_store_retrieve(store, nid);
if (alg == NULL) {
ossl_property_unlock(store);
return 0;
}
if (alg == NULL)
goto err;
if (method == NULL) {
elem.query = prop_query;
if (lh_QUERY_delete(alg->cache, &elem) != NULL)
if ((old = lh_QUERY_delete(alg->cache, &elem)) != NULL) {
impl_cache_free(old);
store->nelem--;
ossl_property_unlock(store);
return 1;
}
goto end;
}
p = OPENSSL_malloc(sizeof(*p) + (len = strlen(prop_query)));
if (p != NULL) {
p->query = p->body;
p->method = method;
p->method.method = method;
p->method.up_ref = method_up_ref;
p->method.free = method_destruct;
if (!ossl_method_up_ref(&p->method))
goto err;
memcpy((char *)p->query, prop_query, len + 1);
if ((old = lh_QUERY_insert(alg->cache, p)) != NULL) {
OPENSSL_free(old);
ossl_property_unlock(store);
return 1;
impl_cache_free(old);
goto end;
}
if (!lh_QUERY_error(alg->cache)) {
if (++store->nelem >= IMPL_CACHE_FLUSH_THRESHOLD)
store->need_flush = 1;
ossl_property_unlock(store);
return 1;
goto end;
}
ossl_method_free(&p->method);
}
ossl_property_unlock(store);
OPENSSL_free(p);
return 0;
err:
res = 0;
end:
ossl_property_unlock(store);
return res;
}

7
doc/internal/man3/OSSL_METHOD_STORE.pod
View File

@ -33,7 +33,9 @@ ossl_method_store_cache_get, ossl_method_store_cache_set
int ossl_method_store_cache_get(OSSL_METHOD_STORE *store, int nid,
const char *prop_query, void **method);
int ossl_method_store_cache_set(OSSL_METHOD_STORE *store, int nid,
const char *prop_query, void *method);
const char *prop_query, void *method,
int (*method_up_ref)(void *),
void (*method_destruct)(void *));
=head1 DESCRIPTION
@ -95,6 +97,9 @@ The result, if any, is returned in I<method>.
ossl_method_store_cache_set() sets a cache entry identified by I<nid> with the
property query I<prop_query> in the I<store>.
Future calls to ossl_method_store_cache_get() will return the specified I<method>.
The I<method_up_ref> function is called to increment the
reference count of the method and the I<method_destruct> function is called
to decrement it.
=head1 RETURN VALUES

4
include/internal/property.h
View File

@ -36,5 +36,7 @@ int ossl_method_store_set_global_properties(OSSL_METHOD_STORE *store,
int ossl_method_store_cache_get(OSSL_METHOD_STORE *store, int nid,
const char *prop_query, void **result);
int ossl_method_store_cache_set(OSSL_METHOD_STORE *store, int nid,
const char *prop_query, void *result);
const char *prop_query, void *result,
int (*method_up_ref)(void *),
void (*method_destruct)(void *));
#endif

21
test/property_test.c
View File

@ -28,6 +28,15 @@ static int add_property_names(const char *n, ...)
return res;
}
static int up_ref(void *p)
{
return 1;
}
static void down_ref(void *p)
{
}
static int test_property_string(void)
{
OSSL_METHOD_STORE *store;
@ -242,7 +251,7 @@ static int test_register_deregister(void)
for (i = 0; i < OSSL_NELEM(impls); i++)
if (!TEST_true(ossl_method_store_add(store, NULL, impls[i].nid,
impls[i].prop, impls[i].impl,
NULL, NULL))) {
&up_ref, &down_ref))) {
TEST_note("iteration %zd", i + 1);
goto err;
}
@ -310,7 +319,7 @@ static int test_property(void)
for (i = 0; i < OSSL_NELEM(impls); i++)
if (!TEST_true(ossl_method_store_add(store, NULL, impls[i].nid,
impls[i].prop, impls[i].impl,
NULL, NULL))) {
&up_ref, &down_ref))) {
TEST_note("iteration %zd", i + 1);
goto err;
}
@ -350,10 +359,12 @@ static int test_query_cache_stochastic(void)
v[i] = 2 * i;
BIO_snprintf(buf, sizeof(buf), "n=%d\n", i);
if (!TEST_true(ossl_method_store_add(store, NULL, i, buf, "abc",
NULL, NULL))
|| !TEST_true(ossl_method_store_cache_set(store, i, buf, v + i))
&up_ref, &down_ref))
|| !TEST_true(ossl_method_store_cache_set(store, i, buf, v + i,
&up_ref, &down_ref))
|| !TEST_true(ossl_method_store_cache_set(store, i, "n=1234",
"miss"))) {
"miss", &up_ref,
&down_ref))) {
TEST_note("iteration %d", i);
goto err;
}