mirror of https://github.com/openssl/openssl
namemap: change ossl_namemap_empty() to do what the documentation says.
The function is documented as returning 1 when passed a NULL argument. Instead it core dumps. Added a unit test for this. Additionally, a performance improvement is incorporated. The namemap max_number field is only ever compared against zero and incremented. The zero comparison grabs a lock specifically for this check. This change uses TSAN operations instead if they are available. Reviewed-by: Richard Levitte <levitte@openssl.org> (Merged from https://github.com/openssl/openssl/pull/12181)
This commit is contained in:
parent
7cc5e0d283
commit
c720fc35f4
|
@ -11,6 +11,7 @@
|
|||
#include "internal/namemap.h"
|
||||
#include <openssl/lhash.h>
|
||||
#include "crypto/lhash.h" /* openssl_lh_strcasehash */
|
||||
#include "internal/tsan_assist.h"
|
||||
|
||||
/*-
|
||||
* The namenum entry
|
||||
|
@ -34,7 +35,12 @@ struct ossl_namemap_st {
|
|||
|
||||
CRYPTO_RWLOCK *lock;
|
||||
LHASH_OF(NAMENUM_ENTRY) *namenum; /* Name->number mapping */
|
||||
int max_number; /* Current max number */
|
||||
|
||||
#ifdef tsan_ld_acq
|
||||
TSAN_QUALIFIER int max_number; /* Current max number TSAN version */
|
||||
#else
|
||||
int max_number; /* Current max number plain version */
|
||||
#endif
|
||||
};
|
||||
|
||||
/* LHASH callbacks */
|
||||
|
@ -91,14 +97,21 @@ static const OPENSSL_CTX_METHOD stored_namemap_method = {
|
|||
|
||||
int ossl_namemap_empty(OSSL_NAMEMAP *namemap)
|
||||
{
|
||||
int rv = 0;
|
||||
#ifdef tsan_ld_acq
|
||||
/* Have TSAN support */
|
||||
return namemap == NULL || tsan_load(&namemap->max_number) == 0;
|
||||
#else
|
||||
/* No TSAN support */
|
||||
int rv;
|
||||
|
||||
if (namemap == NULL)
|
||||
return 1;
|
||||
|
||||
CRYPTO_THREAD_read_lock(namemap->lock);
|
||||
if (namemap->max_number == 0)
|
||||
rv = 1;
|
||||
rv = namemap->max_number == 0;
|
||||
CRYPTO_THREAD_unlock(namemap->lock);
|
||||
|
||||
return rv;
|
||||
#endif
|
||||
}
|
||||
|
||||
typedef struct doall_names_data_st {
|
||||
|
@ -216,7 +229,7 @@ int ossl_namemap_add_name_n(OSSL_NAMEMAP *namemap, int number,
|
|||
goto err;
|
||||
|
||||
namenum->number = tmp_number =
|
||||
number != 0 ? number : ++namemap->max_number;
|
||||
number != 0 ? number : 1 + tsan_counter(&namemap->max_number);
|
||||
(void)lh_NAMENUM_ENTRY_insert(namemap->namenum, namenum);
|
||||
|
||||
if (lh_NAMENUM_ENTRY_error(namemap->namenum))
|
||||
|
|
|
@ -16,6 +16,20 @@
|
|||
#define ALIAS1 "alias1"
|
||||
#define ALIAS1_UC "ALIAS1"
|
||||
|
||||
static int test_namemap_empty(void)
|
||||
{
|
||||
OSSL_NAMEMAP *nm = NULL;
|
||||
int ok;
|
||||
|
||||
ok = TEST_true(ossl_namemap_empty(NULL))
|
||||
&& TEST_ptr(nm = ossl_namemap_new())
|
||||
&& TEST_true(ossl_namemap_empty(nm))
|
||||
&& TEST_int_ne(ossl_namemap_add_name(nm, 0, NAME1), 0)
|
||||
&& TEST_false(ossl_namemap_empty(nm));
|
||||
ossl_namemap_free(nm);
|
||||
return ok;
|
||||
}
|
||||
|
||||
static int test_namemap(OSSL_NAMEMAP *nm)
|
||||
{
|
||||
int num1 = ossl_namemap_add_name(nm, 0, NAME1);
|
||||
|
@ -42,7 +56,7 @@ static int test_namemap(OSSL_NAMEMAP *nm)
|
|||
static int test_namemap_independent(void)
|
||||
{
|
||||
OSSL_NAMEMAP *nm = ossl_namemap_new();
|
||||
int ok = nm != NULL && test_namemap(nm);
|
||||
int ok = TEST_ptr(nm) && test_namemap(nm);
|
||||
|
||||
ossl_namemap_free(nm);
|
||||
return ok;
|
||||
|
@ -52,7 +66,7 @@ static int test_namemap_stored(void)
|
|||
{
|
||||
OSSL_NAMEMAP *nm = ossl_namemap_stored(NULL);
|
||||
|
||||
return nm != NULL
|
||||
return TEST_ptr(nm)
|
||||
&& test_namemap(nm);
|
||||
}
|
||||
|
||||
|
@ -66,6 +80,8 @@ static int test_digestbyname(void)
|
|||
OSSL_NAMEMAP *nm = ossl_namemap_stored(NULL);
|
||||
const EVP_MD *sha256, *foo;
|
||||
|
||||
if (!TEST_ptr(nm))
|
||||
return 0;
|
||||
id = ossl_namemap_add_name(nm, 0, "SHA256");
|
||||
if (!TEST_int_ne(id, 0))
|
||||
return 0;
|
||||
|
@ -92,6 +108,8 @@ static int test_cipherbyname(void)
|
|||
OSSL_NAMEMAP *nm = ossl_namemap_stored(NULL);
|
||||
const EVP_CIPHER *aes128, *bar;
|
||||
|
||||
if (!TEST_ptr(nm))
|
||||
return 0;
|
||||
id = ossl_namemap_add_name(nm, 0, "AES-128-CBC");
|
||||
if (!TEST_int_ne(id, 0))
|
||||
return 0;
|
||||
|
@ -117,7 +135,7 @@ static int test_cipher_is_a(void)
|
|||
EVP_CIPHER *fetched = EVP_CIPHER_fetch(NULL, "AES-256-CCM", NULL);
|
||||
int rv = 1;
|
||||
|
||||
if (!TEST_ptr_ne(fetched, NULL))
|
||||
if (!TEST_ptr(fetched))
|
||||
return 0;
|
||||
if (!TEST_true(EVP_CIPHER_is_a(fetched, "id-aes256-CCM"))
|
||||
|| !TEST_false(EVP_CIPHER_is_a(fetched, "AES-128-GCM")))
|
||||
|
@ -139,7 +157,7 @@ static int test_digest_is_a(void)
|
|||
EVP_MD *fetched = EVP_MD_fetch(NULL, "SHA2-512", NULL);
|
||||
int rv = 1;
|
||||
|
||||
if (!TEST_ptr_ne(fetched, NULL))
|
||||
if (!TEST_ptr(fetched))
|
||||
return 0;
|
||||
if (!TEST_true(EVP_MD_is_a(fetched, "SHA512"))
|
||||
|| !TEST_false(EVP_MD_is_a(fetched, "SHA1")))
|
||||
|
@ -154,6 +172,7 @@ static int test_digest_is_a(void)
|
|||
|
||||
int setup_tests(void)
|
||||
{
|
||||
ADD_TEST(test_namemap_empty);
|
||||
ADD_TEST(test_namemap_independent);
|
||||
ADD_TEST(test_namemap_stored);
|
||||
ADD_TEST(test_digestbyname);
|
||||
|
|
Loading…
Reference in New Issue