[openssl-commits] [openssl] master update
Rich Salz
rsalz at openssl.org
Wed Jun 7 17:06:25 UTC 2017
The branch master has been updated
via be606c013d31847718ceb5d97c567988a771c2e5 (commit)
from db0f35dda18403accabe98e7780f3dfc516f49de (commit)
- Log -----------------------------------------------------------------
commit be606c013d31847718ceb5d97c567988a771c2e5
Author: Rich Salz <rsalz at openssl.org>
Date: Wed Jun 7 11:23:37 2017 -0400
Add a lock around the OBJ_NAME table
Various initialization functions modify this table, which can cause heap
corruption in the absence of external synchronization.
Some stats are modified from OPENSSL_LH_retrieve, where callers aren't
expecting to have to take out an exclusive lock. Switch to using atomic
operations for those stats.
Reviewed-by: Matt Caswell <matt at openssl.org>
Reviewed-by: Rich Salz <rsalz at openssl.org>
(Merged from https://github.com/openssl/openssl/pull/3525)
-----------------------------------------------------------------------
Summary of changes:
crypto/lhash/lh_stats.c | 22 +++++++++---
crypto/lhash/lhash.c | 29 ++++++++-------
crypto/lhash/lhash_lcl.h | 19 ++++++----
crypto/objects/o_names.c | 93 +++++++++++++++++++++++++++++++++---------------
4 files changed, 112 insertions(+), 51 deletions(-)
diff --git a/crypto/lhash/lh_stats.c b/crypto/lhash/lh_stats.c
index 7337832..5586afa 100644
--- a/crypto/lhash/lh_stats.c
+++ b/crypto/lhash/lh_stats.c
@@ -61,6 +61,9 @@ void OPENSSL_LH_node_usage_stats(const OPENSSL_LHASH *lh, FILE *fp)
void OPENSSL_LH_stats_bio(const OPENSSL_LHASH *lh, BIO *out)
{
+ OPENSSL_LHASH *lh_mut = (OPENSSL_LHASH *) lh;
+ int ret;
+
BIO_printf(out, "num_items = %lu\n", lh->num_items);
BIO_printf(out, "num_nodes = %u\n", lh->num_nodes);
BIO_printf(out, "num_alloc_nodes = %u\n", lh->num_alloc_nodes);
@@ -69,15 +72,24 @@ void OPENSSL_LH_stats_bio(const OPENSSL_LHASH *lh, BIO *out)
BIO_printf(out, "num_contracts = %lu\n", lh->num_contracts);
BIO_printf(out, "num_contract_reallocs = %lu\n",
lh->num_contract_reallocs);
- BIO_printf(out, "num_hash_calls = %lu\n", lh->num_hash_calls);
- BIO_printf(out, "num_comp_calls = %lu\n", lh->num_comp_calls);
+ CRYPTO_atomic_add(&lh_mut->num_hash_calls, 0, &ret,
+ lh->retrieve_stats_lock);
+ BIO_printf(out, "num_hash_calls = %d\n", ret);
+ CRYPTO_atomic_add(&lh_mut->num_comp_calls, 0, &ret,
+ lh->retrieve_stats_lock);
+ BIO_printf(out, "num_comp_calls = %d\n", ret);
BIO_printf(out, "num_insert = %lu\n", lh->num_insert);
BIO_printf(out, "num_replace = %lu\n", lh->num_replace);
BIO_printf(out, "num_delete = %lu\n", lh->num_delete);
BIO_printf(out, "num_no_delete = %lu\n", lh->num_no_delete);
- BIO_printf(out, "num_retrieve = %lu\n", lh->num_retrieve);
- BIO_printf(out, "num_retrieve_miss = %lu\n", lh->num_retrieve_miss);
- BIO_printf(out, "num_hash_comps = %lu\n", lh->num_hash_comps);
+ CRYPTO_atomic_add(&lh_mut->num_retrieve, 0, &ret, lh->retrieve_stats_lock);
+ BIO_printf(out, "num_retrieve = %d\n", ret);
+ CRYPTO_atomic_add(&lh_mut->num_retrieve_miss, 0, &ret,
+ lh->retrieve_stats_lock);
+ BIO_printf(out, "num_retrieve_miss = %d\n", ret);
+ CRYPTO_atomic_add(&lh_mut->num_hash_comps, 0, &ret,
+ lh->retrieve_stats_lock);
+ BIO_printf(out, "num_hash_comps = %d\n", ret);
}
void OPENSSL_LH_node_stats_bio(const OPENSSL_LHASH *lh, BIO *out)
diff --git a/crypto/lhash/lhash.c b/crypto/lhash/lhash.c
index 82de223..0fbd385 100644
--- a/crypto/lhash/lhash.c
+++ b/crypto/lhash/lhash.c
@@ -29,9 +29,11 @@ OPENSSL_LHASH *OPENSSL_LH_new(OPENSSL_LH_HASHFUNC h, OPENSSL_LH_COMPFUNC c)
OPENSSL_LHASH *ret;
if ((ret = OPENSSL_zalloc(sizeof(*ret))) == NULL)
- goto err0;
+ return NULL;
if ((ret->b = OPENSSL_zalloc(sizeof(*ret->b) * MIN_NODES)) == NULL)
- goto err1;
+ goto err;
+ if ((ret->retrieve_stats_lock = CRYPTO_THREAD_lock_new()) == NULL)
+ goto err;
ret->comp = ((c == NULL) ? (OPENSSL_LH_COMPFUNC)strcmp : c);
ret->hash = ((h == NULL) ? (OPENSSL_LH_HASHFUNC)OPENSSL_LH_strhash : h);
ret->num_nodes = MIN_NODES / 2;
@@ -41,10 +43,10 @@ OPENSSL_LHASH *OPENSSL_LH_new(OPENSSL_LH_HASHFUNC h, OPENSSL_LH_COMPFUNC c)
ret->down_load = DOWN_LOAD;
return (ret);
- err1:
+err:
+ OPENSSL_free(ret->b);
OPENSSL_free(ret);
- err0:
- return (NULL);
+ return NULL;
}
void OPENSSL_LH_free(OPENSSL_LHASH *lh)
@@ -63,6 +65,7 @@ void OPENSSL_LH_free(OPENSSL_LHASH *lh)
n = nn;
}
}
+ CRYPTO_THREAD_lock_free(lh->retrieve_stats_lock);
OPENSSL_free(lh->b);
OPENSSL_free(lh);
}
@@ -133,18 +136,19 @@ void *OPENSSL_LH_retrieve(OPENSSL_LHASH *lh, const void *data)
unsigned long hash;
OPENSSL_LH_NODE **rn;
void *ret;
+ int scratch;
lh->error = 0;
rn = getrn(lh, data, &hash);
if (*rn == NULL) {
- lh->num_retrieve_miss++;
- return (NULL);
+ CRYPTO_atomic_add(&lh->num_retrieve_miss, 1, &scratch, lh->retrieve_stats_lock);
+ return NULL;
} else {
ret = (*rn)->data;
- lh->num_retrieve++;
+ CRYPTO_atomic_add(&lh->num_retrieve, 1, &scratch, lh->retrieve_stats_lock);
}
- return (ret);
+ return ret;
}
static void doall_util_fn(OPENSSL_LHASH *lh, int use_arg,
@@ -270,9 +274,10 @@ static OPENSSL_LH_NODE **getrn(OPENSSL_LHASH *lh,
OPENSSL_LH_NODE **ret, *n1;
unsigned long hash, nn;
OPENSSL_LH_COMPFUNC cf;
+ int scratch;
hash = (*(lh->hash)) (data);
- lh->num_hash_calls++;
+ CRYPTO_atomic_add(&lh->num_hash_calls, 1, &scratch, lh->retrieve_stats_lock);
*rhash = hash;
nn = hash % lh->pmax;
@@ -282,12 +287,12 @@ static OPENSSL_LH_NODE **getrn(OPENSSL_LHASH *lh,
cf = lh->comp;
ret = &(lh->b[(int)nn]);
for (n1 = *ret; n1 != NULL; n1 = n1->next) {
- lh->num_hash_comps++;
+ CRYPTO_atomic_add(&lh->num_hash_comps, 1, &scratch, lh->retrieve_stats_lock);
if (n1->hash != hash) {
ret = &(n1->next);
continue;
}
- lh->num_comp_calls++;
+ CRYPTO_atomic_add(&lh->num_comp_calls, 1, &scratch, lh->retrieve_stats_lock);
if (cf(n1->data, data) == 0)
break;
ret = &(n1->next);
diff --git a/crypto/lhash/lhash_lcl.h b/crypto/lhash/lhash_lcl.h
index eb4a1a3..01d463f 100644
--- a/crypto/lhash/lhash_lcl.h
+++ b/crypto/lhash/lhash_lcl.h
@@ -6,7 +6,7 @@
* in the file LICENSE in the source distribution or at
* https://www.openssl.org/source/license.html
*/
-
+#include <openssl/crypto.h>
struct lhash_node_st {
void *data;
@@ -18,6 +18,13 @@ struct lhash_st {
OPENSSL_LH_NODE **b;
OPENSSL_LH_COMPFUNC comp;
OPENSSL_LH_HASHFUNC hash;
+ /*
+ * some stats are updated on lookup, which callers aren't expecting to have
+ * to take an exclusive lock around. This lock protects them on platforms
+ * without atomics, and their types are int rather than unsigned long below
+ * so they can be adjusted with CRYPTO_atomic_add.
+ */
+ CRYPTO_RWLOCK *retrieve_stats_lock;
unsigned int num_nodes;
unsigned int num_alloc_nodes;
unsigned int p;
@@ -29,14 +36,14 @@ struct lhash_st {
unsigned long num_expand_reallocs;
unsigned long num_contracts;
unsigned long num_contract_reallocs;
- unsigned long num_hash_calls;
- unsigned long num_comp_calls;
+ int num_hash_calls;
+ int num_comp_calls;
unsigned long num_insert;
unsigned long num_replace;
unsigned long num_delete;
unsigned long num_no_delete;
- unsigned long num_retrieve;
- unsigned long num_retrieve_miss;
- unsigned long num_hash_comps;
+ int num_retrieve;
+ int num_retrieve_miss;
+ int num_hash_comps;
int error;
};
diff --git a/crypto/objects/o_names.c b/crypto/objects/o_names.c
index ed98df8..15fe653 100644
--- a/crypto/objects/o_names.c
+++ b/crypto/objects/o_names.c
@@ -16,6 +16,7 @@
#include <openssl/objects.h>
#include <openssl/safestack.h>
#include <openssl/e_os2.h>
+#include <internal/thread_once.h>
#include "obj_lcl.h"
/*
@@ -44,6 +45,7 @@ static int obj_strcmp(const char *a, const char *b)
*/
static LHASH_OF(OBJ_NAME) *names_lh = NULL;
static int names_type_num = OBJ_NAME_TYPE_NUM;
+static CRYPTO_RWLOCK *lock = NULL;
struct name_funcs_st {
unsigned long (*hash_func) (const char *name);
@@ -62,23 +64,33 @@ static STACK_OF(NAME_FUNCS) *name_funcs_stack;
static unsigned long obj_name_hash(const OBJ_NAME *a);
static int obj_name_cmp(const OBJ_NAME *a, const OBJ_NAME *b);
-int OBJ_NAME_init(void)
+static CRYPTO_ONCE init = CRYPTO_ONCE_STATIC_INIT;
+DEFINE_RUN_ONCE_STATIC(o_names_init)
{
- if (names_lh != NULL)
- return (1);
CRYPTO_mem_ctrl(CRYPTO_MEM_CHECK_DISABLE);
names_lh = lh_OBJ_NAME_new(obj_name_hash, obj_name_cmp);
+ lock = CRYPTO_THREAD_lock_new();
CRYPTO_mem_ctrl(CRYPTO_MEM_CHECK_ENABLE);
- return (names_lh != NULL);
+ return names_lh != NULL && lock != NULL;
+}
+
+int OBJ_NAME_init(void)
+{
+ return RUN_ONCE(&init, o_names_init);
}
int OBJ_NAME_new_index(unsigned long (*hash_func) (const char *),
int (*cmp_func) (const char *, const char *),
void (*free_func) (const char *, int, const char *))
{
- int ret, i, push;
+ int ret = 0, i, push;
NAME_FUNCS *name_funcs;
+ if (!OBJ_NAME_init())
+ return 0;
+
+ CRYPTO_THREAD_write_lock(lock);
+
if (name_funcs_stack == NULL) {
CRYPTO_mem_ctrl(CRYPTO_MEM_CHECK_DISABLE);
name_funcs_stack = sk_NAME_FUNCS_new_null();
@@ -86,7 +98,7 @@ int OBJ_NAME_new_index(unsigned long (*hash_func) (const char *),
}
if (name_funcs_stack == NULL) {
/* ERROR */
- return (0);
+ goto out;
}
ret = names_type_num;
names_type_num++;
@@ -96,7 +108,8 @@ int OBJ_NAME_new_index(unsigned long (*hash_func) (const char *),
CRYPTO_mem_ctrl(CRYPTO_MEM_CHECK_ENABLE);
if (name_funcs == NULL) {
OBJerr(OBJ_F_OBJ_NAME_NEW_INDEX, ERR_R_MALLOC_FAILURE);
- return (0);
+ ret = 0;
+ goto out;
}
name_funcs->hash_func = OPENSSL_LH_strhash;
name_funcs->cmp_func = obj_strcmp;
@@ -108,7 +121,8 @@ int OBJ_NAME_new_index(unsigned long (*hash_func) (const char *),
if (!push) {
OBJerr(OBJ_F_OBJ_NAME_NEW_INDEX, ERR_R_MALLOC_FAILURE);
OPENSSL_free(name_funcs);
- return 0;
+ ret = 0;
+ goto out;
}
}
name_funcs = sk_NAME_FUNCS_value(name_funcs_stack, ret);
@@ -118,7 +132,10 @@ int OBJ_NAME_new_index(unsigned long (*hash_func) (const char *),
name_funcs->cmp_func = cmp_func;
if (free_func != NULL)
name_funcs->free_func = free_func;
- return (ret);
+
+out:
+ CRYPTO_THREAD_unlock(lock);
+ return ret;
}
static int obj_name_cmp(const OBJ_NAME *a, const OBJ_NAME *b)
@@ -134,7 +151,7 @@ static int obj_name_cmp(const OBJ_NAME *a, const OBJ_NAME *b)
} else
ret = strcmp(a->name, b->name);
}
- return (ret);
+ return ret;
}
static unsigned long obj_name_hash(const OBJ_NAME *a)
@@ -150,18 +167,20 @@ static unsigned long obj_name_hash(const OBJ_NAME *a)
ret = OPENSSL_LH_strhash(a->name);
}
ret ^= a->type;
- return (ret);
+ return ret;
}
const char *OBJ_NAME_get(const char *name, int type)
{
OBJ_NAME on, *ret;
int num = 0, alias;
+ const char *value = NULL;
if (name == NULL)
- return (NULL);
- if ((names_lh == NULL) && !OBJ_NAME_init())
- return (NULL);
+ return NULL;
+ if (!OBJ_NAME_init())
+ return NULL;
+ CRYPTO_THREAD_read_lock(lock);
alias = type & OBJ_NAME_ALIAS;
type &= ~OBJ_NAME_ALIAS;
@@ -172,24 +191,30 @@ const char *OBJ_NAME_get(const char *name, int type)
for (;;) {
ret = lh_OBJ_NAME_retrieve(names_lh, &on);
if (ret == NULL)
- return (NULL);
+ break;
if ((ret->alias) && !alias) {
if (++num > 10)
- return (NULL);
+ break;
on.name = ret->data;
} else {
- return (ret->data);
+ value = ret->data;
+ break;
}
}
+
+ CRYPTO_THREAD_unlock(lock);
+ return value;
}
int OBJ_NAME_add(const char *name, int type, const char *data)
{
OBJ_NAME *onp, *ret;
- int alias;
+ int alias, ok = 0;
- if ((names_lh == NULL) && !OBJ_NAME_init())
- return (0);
+ if (!OBJ_NAME_init())
+ return 0;
+
+ CRYPTO_THREAD_write_lock(lock);
alias = type & OBJ_NAME_ALIAS;
type &= ~OBJ_NAME_ALIAS;
@@ -197,7 +222,7 @@ int OBJ_NAME_add(const char *name, int type, const char *data)
onp = OPENSSL_malloc(sizeof(*onp));
if (onp == NULL) {
/* ERROR */
- return 0;
+ goto unlock;
}
onp->name = name;
@@ -223,18 +248,26 @@ int OBJ_NAME_add(const char *name, int type, const char *data)
if (lh_OBJ_NAME_error(names_lh)) {
/* ERROR */
OPENSSL_free(onp);
- return 0;
+ goto unlock;
}
}
- return 1;
+
+ ok = 1;
+
+unlock:
+ CRYPTO_THREAD_unlock(lock);
+ return ok;
}
int OBJ_NAME_remove(const char *name, int type)
{
OBJ_NAME on, *ret;
+ int ok = 0;
- if (names_lh == NULL)
- return (0);
+ if (!OBJ_NAME_init())
+ return 0;
+
+ CRYPTO_THREAD_write_lock(lock);
type &= ~OBJ_NAME_ALIAS;
on.name = name;
@@ -253,9 +286,11 @@ int OBJ_NAME_remove(const char *name, int type)
ret->data);
}
OPENSSL_free(ret);
- return (1);
- } else
- return (0);
+ ok = 1;
+ }
+
+ CRYPTO_THREAD_unlock(lock);
+ return ok;
}
typedef struct {
@@ -363,8 +398,10 @@ void OBJ_NAME_cleanup(int type)
if (type < 0) {
lh_OBJ_NAME_free(names_lh);
sk_NAME_FUNCS_pop_free(name_funcs_stack, name_funcs_free);
+ CRYPTO_THREAD_lock_free(lock);
names_lh = NULL;
name_funcs_stack = NULL;
+ lock = NULL;
} else
lh_OBJ_NAME_set_down_load(names_lh, down_load);
}
More information about the openssl-commits
mailing list