Remove the CRYPTO_EX_new callback.

This callback is never used. The one caller I've ever seen is in Android
code which isn't built with BoringSSL and it was a no-op.

It also doesn't actually make much sense. A callback cannot reasonably
assume that it sees every, say, SSL_CTX created because the index may be
registered after the first SSL_CTX is created. Nor is there any point in
an EX_DATA consumer in one file knowing about an SSL_CTX created in
completely unrelated code.

Replace all the pointers with a typedef to int*. This will ensure code
which passes NULL or 0 continues to compile while breaking code which
passes an actual function.

This simplifies some object creation functions which now needn't worry
about CRYPTO_new_ex_data failing. (Also avoids bouncing on the lock, but
it's taking a read lock, so this doesn't really matter.)

BUG=391192

Change-Id: I02893883c6fa8693682075b7b130aa538a0a1437
Reviewed-on: https://boringssl-review.googlesource.com/6625
Reviewed-by: Adam Langley <agl@google.com>
This commit is contained in:
David Benjamin 2015-12-04 23:14:35 -05:00 committed by Adam Langley
parent 0abd6f2eb6
commit 8a58933db0
18 changed files with 68 additions and 127 deletions

View File

@ -85,11 +85,7 @@ DH *DH_new(void) {
CRYPTO_MUTEX_init(&dh->method_mont_p_lock);
dh->references = 1;
if (!CRYPTO_new_ex_data(&g_ex_data_class, dh, &dh->ex_data)) {
CRYPTO_MUTEX_cleanup(&dh->method_mont_p_lock);
OPENSSL_free(dh);
return NULL;
}
CRYPTO_new_ex_data(&dh->ex_data);
return dh;
}
@ -448,11 +444,11 @@ DH *DHparams_dup(const DH *dh) {
return ret;
}
int DH_get_ex_new_index(long argl, void *argp, CRYPTO_EX_new *new_func,
int DH_get_ex_new_index(long argl, void *argp, CRYPTO_EX_unused *unused,
CRYPTO_EX_dup *dup_func, CRYPTO_EX_free *free_func) {
int index;
if (!CRYPTO_get_ex_new_index(&g_ex_data_class, &index, argl, argp, new_func,
dup_func, free_func)) {
if (!CRYPTO_get_ex_new_index(&g_ex_data_class, &index, argl, argp, dup_func,
free_func)) {
return -1;
}
return index;

View File

@ -98,12 +98,7 @@ DSA *DSA_new(void) {
dsa->references = 1;
CRYPTO_MUTEX_init(&dsa->method_mont_p_lock);
if (!CRYPTO_new_ex_data(&g_ex_data_class, dsa, &dsa->ex_data)) {
CRYPTO_MUTEX_cleanup(&dsa->method_mont_p_lock);
OPENSSL_free(dsa);
return NULL;
}
CRYPTO_new_ex_data(&dsa->ex_data);
return dsa;
}
@ -864,11 +859,11 @@ err:
return ret;
}
int DSA_get_ex_new_index(long argl, void *argp, CRYPTO_EX_new *new_func,
int DSA_get_ex_new_index(long argl, void *argp, CRYPTO_EX_unused *unused,
CRYPTO_EX_dup *dup_func, CRYPTO_EX_free *free_func) {
int index;
if (!CRYPTO_get_ex_new_index(&g_ex_data_class, &index, argl, argp, new_func,
dup_func, free_func)) {
if (!CRYPTO_get_ex_new_index(&g_ex_data_class, &index, argl, argp, dup_func,
free_func)) {
return -1;
}
return index;

View File

@ -104,19 +104,10 @@ EC_KEY *EC_KEY_new_method(const ENGINE *engine) {
ret->conv_form = POINT_CONVERSION_UNCOMPRESSED;
ret->references = 1;
if (!CRYPTO_new_ex_data(&g_ex_data_class, ret, &ret->ex_data)) {
goto err1;
}
CRYPTO_new_ex_data(&ret->ex_data);
if (ret->ecdsa_meth && ret->ecdsa_meth->init && !ret->ecdsa_meth->init(ret)) {
goto err2;
}
return ret;
err2:
CRYPTO_free_ex_data(&g_ex_data_class, ret, &ret->ex_data);
err1:
if (ret->ecdsa_meth) {
METHOD_unref(ret->ecdsa_meth);
}
@ -124,6 +115,9 @@ err1:
return NULL;
}
return ret;
}
EC_KEY *EC_KEY_new_by_curve_name(int nid) {
EC_KEY *ret = EC_KEY_new();
if (ret == NULL) {
@ -466,12 +460,12 @@ err:
return ok;
}
int EC_KEY_get_ex_new_index(long argl, void *argp, CRYPTO_EX_new *new_func,
int EC_KEY_get_ex_new_index(long argl, void *argp, CRYPTO_EX_unused *unused,
CRYPTO_EX_dup *dup_func,
CRYPTO_EX_free *free_func) {
int index;
if (!CRYPTO_get_ex_new_index(&g_ex_data_class, &index, argl, argp, new_func,
dup_func, free_func)) {
if (!CRYPTO_get_ex_new_index(&g_ex_data_class, &index, argl, argp, dup_func,
free_func)) {
return -1;
}
return index;

View File

@ -124,14 +124,12 @@
struct crypto_ex_data_func_st {
long argl; /* Arbitary long */
void *argp; /* Arbitary void pointer */
CRYPTO_EX_new *new_func;
CRYPTO_EX_free *free_func;
CRYPTO_EX_dup *dup_func;
};
int CRYPTO_get_ex_new_index(CRYPTO_EX_DATA_CLASS *ex_data_class, int *out_index,
long argl, void *argp, CRYPTO_EX_new *new_func,
CRYPTO_EX_dup *dup_func,
long argl, void *argp, CRYPTO_EX_dup *dup_func,
CRYPTO_EX_free *free_func) {
CRYPTO_EX_DATA_FUNCS *funcs;
int ret = 0;
@ -144,7 +142,6 @@ int CRYPTO_get_ex_new_index(CRYPTO_EX_DATA_CLASS *ex_data_class, int *out_index,
funcs->argl = argl;
funcs->argp = argp;
funcs->new_func = new_func;
funcs->dup_func = dup_func;
funcs->free_func = free_func;
@ -230,29 +227,8 @@ static int get_func_pointers(STACK_OF(CRYPTO_EX_DATA_FUNCS) **out,
return 1;
}
int CRYPTO_new_ex_data(CRYPTO_EX_DATA_CLASS *ex_data_class, void *obj,
CRYPTO_EX_DATA *ad) {
STACK_OF(CRYPTO_EX_DATA_FUNCS) *func_pointers;
size_t i;
void CRYPTO_new_ex_data(CRYPTO_EX_DATA *ad) {
ad->sk = NULL;
if (!get_func_pointers(&func_pointers, ex_data_class)) {
return 0;
}
for (i = 0; i < sk_CRYPTO_EX_DATA_FUNCS_num(func_pointers); i++) {
CRYPTO_EX_DATA_FUNCS *func_pointer =
sk_CRYPTO_EX_DATA_FUNCS_value(func_pointers, i);
if (func_pointer->new_func) {
func_pointer->new_func(obj, NULL, ad, i + ex_data_class->num_reserved,
func_pointer->argl, func_pointer->argp);
}
}
sk_CRYPTO_EX_DATA_FUNCS_free(func_pointers);
return 1;
}
int CRYPTO_dup_ex_data(CRYPTO_EX_DATA_CLASS *ex_data_class, CRYPTO_EX_DATA *to,

View File

@ -497,8 +497,7 @@ typedef struct {
* zero otherwise. */
OPENSSL_EXPORT int CRYPTO_get_ex_new_index(CRYPTO_EX_DATA_CLASS *ex_data_class,
int *out_index, long argl,
void *argp, CRYPTO_EX_new *new_func,
CRYPTO_EX_dup *dup_func,
void *argp, CRYPTO_EX_dup *dup_func,
CRYPTO_EX_free *free_func);
/* CRYPTO_set_ex_data sets an extra data pointer on a given object. Each class
@ -510,11 +509,8 @@ OPENSSL_EXPORT int CRYPTO_set_ex_data(CRYPTO_EX_DATA *ad, int index, void *val);
* function. */
OPENSSL_EXPORT void *CRYPTO_get_ex_data(const CRYPTO_EX_DATA *ad, int index);
/* CRYPTO_new_ex_data initialises a newly allocated |CRYPTO_EX_DATA| which is
* embedded inside of |obj| which is of class |ex_data_class|. Returns one on
* success and zero otherwise. */
OPENSSL_EXPORT int CRYPTO_new_ex_data(CRYPTO_EX_DATA_CLASS *ex_data_class,
void *obj, CRYPTO_EX_DATA *ad);
/* CRYPTO_new_ex_data initialises a newly allocated |CRYPTO_EX_DATA|. */
OPENSSL_EXPORT void CRYPTO_new_ex_data(CRYPTO_EX_DATA *ad);
/* CRYPTO_dup_ex_data duplicates |from| into a freshly allocated
* |CRYPTO_EX_DATA|, |to|. Both of which are inside objects of the given

View File

@ -96,13 +96,7 @@ RSA *RSA_new_method(const ENGINE *engine) {
rsa->references = 1;
rsa->flags = rsa->meth->flags;
CRYPTO_MUTEX_init(&rsa->lock);
if (!CRYPTO_new_ex_data(&g_ex_data_class, rsa, &rsa->ex_data)) {
CRYPTO_MUTEX_cleanup(&rsa->lock);
METHOD_unref(rsa->meth);
OPENSSL_free(rsa);
return NULL;
}
CRYPTO_new_ex_data(&rsa->ex_data);
if (rsa->meth->init && !rsa->meth->init(rsa)) {
CRYPTO_free_ex_data(&g_ex_data_class, rsa, &rsa->ex_data);
@ -308,11 +302,11 @@ int RSA_supports_digest(const RSA *rsa, const EVP_MD *md) {
return 1;
}
int RSA_get_ex_new_index(long argl, void *argp, CRYPTO_EX_new *new_func,
int RSA_get_ex_new_index(long argl, void *argp, CRYPTO_EX_unused *unused,
CRYPTO_EX_dup *dup_func, CRYPTO_EX_free *free_func) {
int index;
if (!CRYPTO_get_ex_new_index(&g_ex_data_class, &index, argl, argp, new_func,
dup_func, free_func)) {
if (!CRYPTO_get_ex_new_index(&g_ex_data_class, &index, argl, argp, dup_func,
free_func)) {
return -1;
}
return index;

View File

@ -2092,14 +2092,14 @@ X509_CRL *X509_CRL_diff(X509_CRL *base, X509_CRL *newer,
return NULL;
}
int X509_STORE_CTX_get_ex_new_index(long argl, void *argp, CRYPTO_EX_new *new_func,
int X509_STORE_CTX_get_ex_new_index(long argl, void *argp, CRYPTO_EX_unused *unused,
CRYPTO_EX_dup *dup_func, CRYPTO_EX_free *free_func)
{
/* This function is (usually) called only once, by
* SSL_get_ex_data_X509_STORE_CTX_idx (ssl/ssl_cert.c). */
int index;
if (!CRYPTO_get_ex_new_index(&g_ex_data_class, &index, argl, argp,
new_func, dup_func, free_func))
dup_func, free_func))
{
return -1;
}
@ -2266,19 +2266,13 @@ int X509_STORE_CTX_init(X509_STORE_CTX *ctx, X509_STORE *store, X509 *x509,
STACK_OF(X509) *chain)
{
int ret = 1;
int ex_data_allocated = 0;
memset(ctx, 0, sizeof(X509_STORE_CTX));
ctx->ctx=store;
ctx->cert=x509;
ctx->untrusted=chain;
if(!CRYPTO_new_ex_data(&g_ex_data_class, ctx,
&ctx->ex_data))
{
goto err;
}
ex_data_allocated = 1;
CRYPTO_new_ex_data(&ctx->ex_data);
ctx->param = X509_VERIFY_PARAM_new();
if (!ctx->param)
@ -2362,10 +2356,7 @@ int X509_STORE_CTX_init(X509_STORE_CTX *ctx, X509_STORE *store, X509 *x509,
return 1;
err:
if (ex_data_allocated)
{
CRYPTO_free_ex_data(&g_ex_data_class, ctx, &ctx->ex_data);
}
if (ctx->param != NULL)
{
X509_VERIFY_PARAM_free(ctx->param);

View File

@ -104,7 +104,7 @@ static int x509_cb(int operation, ASN1_VALUE **pval, const ASN1_ITEM *it,
ret->akid = NULL;
ret->aux = NULL;
ret->crldp = NULL;
CRYPTO_new_ex_data(&g_ex_data_class, ret, &ret->ex_data);
CRYPTO_new_ex_data(&ret->ex_data);
break;
case ASN1_OP_D2I_POST:
@ -146,12 +146,12 @@ X509 *X509_up_ref(X509 *x)
return x;
}
int X509_get_ex_new_index(long argl, void *argp, CRYPTO_EX_new *new_func,
int X509_get_ex_new_index(long argl, void *argp, CRYPTO_EX_unused *unused,
CRYPTO_EX_dup *dup_func, CRYPTO_EX_free *free_func)
{
int index;
if (!CRYPTO_get_ex_new_index(&g_ex_data_class, &index, argl, argp,
new_func, dup_func, free_func))
dup_func, free_func))
{
return -1;
}

View File

@ -193,7 +193,7 @@ OPENSSL_EXPORT int i2d_DHparams(const DH *in, unsigned char **outp);
* See |ex_data.h| for details. */
OPENSSL_EXPORT int DH_get_ex_new_index(long argl, void *argp,
CRYPTO_EX_new *new_func,
CRYPTO_EX_unused *unused,
CRYPTO_EX_dup *dup_func,
CRYPTO_EX_free *free_func);
OPENSSL_EXPORT int DH_set_ex_data(DH *d, int idx, void *arg);

View File

@ -302,7 +302,7 @@ OPENSSL_EXPORT DH *DSA_dup_DH(const DSA *dsa);
* See |ex_data.h| for details. */
OPENSSL_EXPORT int DSA_get_ex_new_index(long argl, void *argp,
CRYPTO_EX_new *new_func,
CRYPTO_EX_unused *unused,
CRYPTO_EX_dup *dup_func,
CRYPTO_EX_free *free_func);
OPENSSL_EXPORT int DSA_set_ex_data(DSA *d, int idx, void *arg);

View File

@ -226,7 +226,7 @@ OPENSSL_EXPORT int i2o_ECPublicKey(const EC_KEY *key, unsigned char **outp);
* These functions are wrappers. See |ex_data.h| for details. */
OPENSSL_EXPORT int EC_KEY_get_ex_new_index(long argl, void *argp,
CRYPTO_EX_new *new_func,
CRYPTO_EX_unused *unused,
CRYPTO_EX_dup *dup_func,
CRYPTO_EX_free *free_func);
OPENSSL_EXPORT int EC_KEY_set_ex_data(EC_KEY *r, int idx, void *arg);

View File

@ -121,8 +121,8 @@ extern "C" {
/* ex_data is a mechanism for associating arbitrary extra data with objects.
* For each type of object that supports ex_data, different users can be
* assigned indexes in which to store their data. Each index has callback
* functions that are called when a new object of that type is created, freed
* and duplicated. */
* functions that are called when an object of that type is freed or
* duplicated. */
typedef struct crypto_ex_data_st CRYPTO_EX_DATA;
@ -142,7 +142,7 @@ typedef struct crypto_ex_data_st CRYPTO_EX_DATA;
*
* TODO(fork): this should follow the standard calling convention. */
OPENSSL_EXPORT int TYPE_get_ex_new_index(long argl, void *argp,
CRYPTO_EX_new *new_func,
CRYPTO_EX_unused *unused,
CRYPTO_EX_dup *dup_func,
CRYPTO_EX_free *free_func);
@ -160,24 +160,15 @@ OPENSSL_EXPORT void *TYPE_get_ex_data(const TYPE *t, int index);
/* Callback types. */
/* CRYPTO_EX_new is the type of a callback function that is called whenever a
* new object of a given class is created. For example, if this callback has
* been passed to |SSL_get_ex_new_index| then it'll be called each time an SSL*
* is created.
/* CRYPTO_EX_free is a callback function that is called when an object of the
* class is being destroyed. For example, if this callback has been passed to
* |SSL_get_ex_new_index| then it'll be called each time an |SSL*| is destroyed.
*
* The callback is passed the new object (i.e. the SSL*) in |parent|. The
* The callback is passed the new object (i.e. the |SSL*|) in |parent|. The
* arguments |argl| and |argp| contain opaque values that were given to
* |CRYPTO_get_ex_new_index|. The callback should return one on success, but
* the value is ignored.
*
* TODO(fork): the |ptr| argument is always NULL, no? */
typedef int CRYPTO_EX_new(void *parent, void *ptr, CRYPTO_EX_DATA *ad,
int index, long argl, void *argp);
/* CRYPTO_EX_free is a callback function that is called when an object of the
* class is being destroyed. See |CRYPTO_EX_new| for a discussion of the
* arguments.
*
* If |CRYPTO_get_ex_new_index| was called after the creation of objects of the
* class that this applies to then, when those those objects are destroyed,
* this callback will be called with a NULL value for |ptr|. */
@ -202,6 +193,13 @@ typedef int CRYPTO_EX_dup(CRYPTO_EX_DATA *to, const CRYPTO_EX_DATA *from,
/* CRYPTO_cleanup_all_ex_data does nothing. */
OPENSSL_EXPORT void CRYPTO_cleanup_all_ex_data(void);
/* Private structures. */
/* CRYPTO_EX_unused is a placeholder for an unused callback. It is aliased to
* int to ensure non-NULL callers fail to compile rather than fail silently. */
typedef int CRYPTO_EX_unused;
struct crypto_ex_data_st {
STACK_OF(void) *sk;
};

View File

@ -386,7 +386,7 @@ OPENSSL_EXPORT int RSA_private_key_to_bytes(uint8_t **out_bytes,
* See |ex_data.h| for details. */
OPENSSL_EXPORT int RSA_get_ex_new_index(long argl, void *argp,
CRYPTO_EX_new *new_func,
CRYPTO_EX_unused *unused,
CRYPTO_EX_dup *dup_func,
CRYPTO_EX_free *free_func);
OPENSSL_EXPORT int RSA_set_ex_data(RSA *r, int idx, void *arg);

View File

@ -2548,7 +2548,7 @@ OPENSSL_EXPORT const char *SSL_alert_desc_string_long(int value);
OPENSSL_EXPORT int SSL_set_ex_data(SSL *ssl, int idx, void *data);
OPENSSL_EXPORT void *SSL_get_ex_data(const SSL *ssl, int idx);
OPENSSL_EXPORT int SSL_get_ex_new_index(long argl, void *argp,
CRYPTO_EX_new *new_func,
CRYPTO_EX_unused *unused,
CRYPTO_EX_dup *dup_func,
CRYPTO_EX_free *free_func);
@ -2557,14 +2557,14 @@ OPENSSL_EXPORT int SSL_SESSION_set_ex_data(SSL_SESSION *session, int idx,
OPENSSL_EXPORT void *SSL_SESSION_get_ex_data(const SSL_SESSION *session,
int idx);
OPENSSL_EXPORT int SSL_SESSION_get_ex_new_index(long argl, void *argp,
CRYPTO_EX_new *new_func,
CRYPTO_EX_unused *unused,
CRYPTO_EX_dup *dup_func,
CRYPTO_EX_free *free_func);
OPENSSL_EXPORT int SSL_CTX_set_ex_data(SSL_CTX *ctx, int idx, void *data);
OPENSSL_EXPORT void *SSL_CTX_get_ex_data(const SSL_CTX *ctx, int idx);
OPENSSL_EXPORT int SSL_CTX_get_ex_new_index(long argl, void *argp,
CRYPTO_EX_new *new_func,
CRYPTO_EX_unused *unused,
CRYPTO_EX_dup *dup_func,
CRYPTO_EX_free *free_func);

View File

@ -787,7 +787,7 @@ DECLARE_ASN1_FUNCTIONS(X509_CERT_PAIR)
* |x|. */
OPENSSL_EXPORT X509 *X509_up_ref(X509 *x);
OPENSSL_EXPORT int X509_get_ex_new_index(long argl, void *argp, CRYPTO_EX_new *new_func,
OPENSSL_EXPORT int X509_get_ex_new_index(long argl, void *argp, CRYPTO_EX_unused *unused,
CRYPTO_EX_dup *dup_func, CRYPTO_EX_free *free_func);
OPENSSL_EXPORT int X509_set_ex_data(X509 *r, int idx, void *arg);
OPENSSL_EXPORT void *X509_get_ex_data(X509 *r, int idx);

View File

@ -498,7 +498,7 @@ OPENSSL_EXPORT int X509_STORE_load_locations (X509_STORE *ctx,
OPENSSL_EXPORT int X509_STORE_set_default_paths(X509_STORE *ctx);
#endif
OPENSSL_EXPORT int X509_STORE_CTX_get_ex_new_index(long argl, void *argp, CRYPTO_EX_new *new_func,
OPENSSL_EXPORT int X509_STORE_CTX_get_ex_new_index(long argl, void *argp, CRYPTO_EX_unused *unused,
CRYPTO_EX_dup *dup_func, CRYPTO_EX_free *free_func);
OPENSSL_EXPORT int X509_STORE_CTX_set_ex_data(X509_STORE_CTX *ctx,int idx,void *data);
OPENSSL_EXPORT void * X509_STORE_CTX_get_ex_data(X509_STORE_CTX *ctx,int idx);

View File

@ -274,7 +274,7 @@ SSL_CTX *SSL_CTX_new(const SSL_METHOD *method) {
goto err;
}
CRYPTO_new_ex_data(&g_ex_data_class_ssl_ctx, ret, &ret->ex_data);
CRYPTO_new_ex_data(&ret->ex_data);
ret->max_send_fragment = SSL3_RT_MAX_PLAIN_LENGTH;
@ -424,7 +424,7 @@ SSL *SSL_new(SSL_CTX *ctx) {
s->rwstate = SSL_NOTHING;
CRYPTO_new_ex_data(&g_ex_data_class_ssl, s, &s->ex_data);
CRYPTO_new_ex_data(&s->ex_data);
s->psk_identity_hint = NULL;
if (ctx->psk_identity_hint) {
@ -2060,11 +2060,11 @@ void SSL_set_verify_result(SSL *ssl, long result) {
long SSL_get_verify_result(const SSL *ssl) { return ssl->verify_result; }
int SSL_get_ex_new_index(long argl, void *argp, CRYPTO_EX_new *new_func,
int SSL_get_ex_new_index(long argl, void *argp, CRYPTO_EX_unused *unused,
CRYPTO_EX_dup *dup_func, CRYPTO_EX_free *free_func) {
int index;
if (!CRYPTO_get_ex_new_index(&g_ex_data_class_ssl, &index, argl, argp,
new_func, dup_func, free_func)) {
dup_func, free_func)) {
return -1;
}
return index;
@ -2078,12 +2078,12 @@ void *SSL_get_ex_data(const SSL *ssl, int idx) {
return CRYPTO_get_ex_data(&ssl->ex_data, idx);
}
int SSL_CTX_get_ex_new_index(long argl, void *argp, CRYPTO_EX_new *new_func,
int SSL_CTX_get_ex_new_index(long argl, void *argp, CRYPTO_EX_unused *unused,
CRYPTO_EX_dup *dup_func,
CRYPTO_EX_free *free_func) {
int index;
if (!CRYPTO_get_ex_new_index(&g_ex_data_class_ssl_ctx, &index, argl, argp,
new_func, dup_func, free_func)) {
dup_func, free_func)) {
return -1;
}
return index;

View File

@ -172,7 +172,7 @@ SSL_SESSION *SSL_SESSION_new(void) {
session->references = 1;
session->timeout = SSL_DEFAULT_SESSION_TIMEOUT;
session->time = (unsigned long)time(NULL);
CRYPTO_new_ex_data(&g_ex_data_class, session, &session->ex_data);
CRYPTO_new_ex_data(&session->ex_data);
return session;
}
@ -278,12 +278,13 @@ SSL_SESSION *SSL_get1_session(SSL *ssl) {
return SSL_SESSION_up_ref(ssl->session);
}
int SSL_SESSION_get_ex_new_index(long argl, void *argp, CRYPTO_EX_new *new_func,
int SSL_SESSION_get_ex_new_index(long argl, void *argp,
CRYPTO_EX_unused *unused,
CRYPTO_EX_dup *dup_func,
CRYPTO_EX_free *free_func) {
int index;
if (!CRYPTO_get_ex_new_index(&g_ex_data_class, &index, argl, argp, new_func,
dup_func, free_func)) {
if (!CRYPTO_get_ex_new_index(&g_ex_data_class, &index, argl, argp, dup_func,
free_func)) {
return -1;
}
return index;