From 62adbcee392ba1061bf213174e8c59728e00860e Mon Sep 17 00:00:00 2001 From: Rich Salz Date: Sat, 11 Apr 2015 10:22:36 -0400 Subject: [PATCH] free NULL cleanup 10 Avoid checking for NULL before calling free functions. This gets ssl.*free: ssl_sess_cert_free ssl_free ssl_excert_free ssl_cert_free SSL_free SSL_SRP_CTX_free SSL_SESSION_free SSL_CTX_free SSL_CTX_SRP_CTX_free SSL_CONF_CTX_free Reviewed-by: Kurt Roeckx --- apps/ciphers.c | 6 ++---- apps/ocsp.c | 3 +-- apps/s_client.c | 6 ++---- apps/s_server.c | 9 +++------ apps/s_time.c | 9 +++------ demos/bio/sconnect.c | 3 +-- demos/easy_tls/easy-tls.c | 3 +-- doc/ssl/SSL_CONF_CTX_new.pod | 1 + doc/ssl/SSL_CTX_free.pod | 2 ++ doc/ssl/SSL_SESSION_free.pod | 1 + doc/ssl/SSL_free.pod | 1 + ssl/bio_ssl.c | 5 ++--- ssl/s3_clnt.c | 3 +-- ssl/ssl_lib.c | 4 ++-- ssl/ssl_sess.c | 9 +++------ test/ssltest.c | 14 +++++--------- 16 files changed, 31 insertions(+), 48 deletions(-) diff --git a/apps/ciphers.c b/apps/ciphers.c index 6c7ff01eea..4b9a114666 100644 --- a/apps/ciphers.c +++ b/apps/ciphers.c @@ -223,10 +223,8 @@ int MAIN(int argc, char **argv) end: if (use_supported && sk) sk_SSL_CIPHER_free(sk); - if (ctx != NULL) - SSL_CTX_free(ctx); - if (ssl != NULL) - SSL_free(ssl); + SSL_CTX_free(ctx); + SSL_free(ssl); BIO_free_all(STDout); apps_shutdown(); OPENSSL_EXIT(ret); diff --git a/apps/ocsp.c b/apps/ocsp.c index 95380964a3..96f4c67421 100644 --- a/apps/ocsp.c +++ b/apps/ocsp.c @@ -1363,8 +1363,7 @@ OCSP_RESPONSE *process_responder(BIO *err, OCSP_REQUEST *req, BIO_printf(bio_err, "Error querying OCSP responder\n"); end: BIO_free_all(cbio); - if (ctx) - SSL_CTX_free(ctx); + SSL_CTX_free(ctx); return resp; } diff --git a/apps/s_client.c b/apps/s_client.c index ec116171c3..a7e03a5f33 100644 --- a/apps/s_client.c +++ b/apps/s_client.c @@ -2024,8 +2024,7 @@ int MAIN(int argc, char **argv) if (next_proto.data) OPENSSL_free(next_proto.data); #endif - if (ctx != NULL) - SSL_CTX_free(ctx); + SSL_CTX_free(ctx); if (cert) X509_free(cert); if (crls) @@ -2040,8 +2039,7 @@ int MAIN(int argc, char **argv) ssl_excert_free(exc); if (ssl_args) sk_OPENSSL_STRING_free(ssl_args); - if (cctx) - SSL_CONF_CTX_free(cctx); + SSL_CONF_CTX_free(cctx); #ifndef OPENSSL_NO_JPAKE if (jpake_secret && psk_key) OPENSSL_free(psk_key); diff --git a/apps/s_server.c b/apps/s_server.c index f97a97d8f0..a66098efe7 100644 --- a/apps/s_server.c +++ b/apps/s_server.c @@ -2003,8 +2003,7 @@ int MAIN(int argc, char *argv[]) print_stats(bio_s_out, ctx); ret = 0; end: - if (ctx != NULL) - SSL_CTX_free(ctx); + SSL_CTX_free(ctx); if (s_cert) X509_free(s_cert); if (crls) @@ -2031,8 +2030,7 @@ int MAIN(int argc, char *argv[]) OPENSSL_free(tlscstatp.port); if (tlscstatp.path) OPENSSL_free(tlscstatp.path); - if (ctx2 != NULL) - SSL_CTX_free(ctx2); + SSL_CTX_free(ctx2); if (s_cert2) X509_free(s_cert2); EVP_PKEY_free(s_key2); @@ -2047,8 +2045,7 @@ int MAIN(int argc, char *argv[]) ssl_excert_free(exc); if (ssl_args) sk_OPENSSL_STRING_free(ssl_args); - if (cctx) - SSL_CONF_CTX_free(cctx); + SSL_CONF_CTX_free(cctx); #ifndef OPENSSL_NO_JPAKE if (jpake_secret && psk_key) OPENSSL_free(psk_key); diff --git a/apps/s_time.c b/apps/s_time.c index 5b94634a53..4f460b6a45 100644 --- a/apps/s_time.c +++ b/apps/s_time.c @@ -540,13 +540,10 @@ int MAIN(int argc, char **argv) ret = 0; end: - if (scon != NULL) - SSL_free(scon); + SSL_free(scon); - if (tm_ctx != NULL) { - SSL_CTX_free(tm_ctx); - tm_ctx = NULL; - } + SSL_CTX_free(tm_ctx); + tm_ctx = NULL; apps_shutdown(); OPENSSL_EXIT(ret); } diff --git a/demos/bio/sconnect.c b/demos/bio/sconnect.c index e6eddb1c05..73280b576a 100644 --- a/demos/bio/sconnect.c +++ b/demos/bio/sconnect.c @@ -106,8 +106,7 @@ char *argv[]; ERR_print_errors_fp(stderr); } BIO_free_all(out); - if (ssl_ctx != NULL) - SSL_CTX_free(ssl_ctx); + SSL_CTX_free(ssl_ctx); exit(!ret); return (ret); } diff --git a/demos/easy_tls/easy-tls.c b/demos/easy_tls/easy-tls.c index 3475551d6a..1a0a03abe6 100644 --- a/demos/easy_tls/easy-tls.c +++ b/demos/easy_tls/easy-tls.c @@ -804,8 +804,7 @@ SSL_CTX *tls_create_ctx(struct tls_create_ctx_args a, void *apparg) err: tls_openssl_errors(err_pref_1, err_pref_2, NULL, apparg); err_return: - if (ret != NULL) - SSL_CTX_free(ret); + SSL_CTX_free(ret); return NULL; } diff --git a/doc/ssl/SSL_CONF_CTX_new.pod b/doc/ssl/SSL_CONF_CTX_new.pod index a9ccb049f4..79c8c94ee0 100644 --- a/doc/ssl/SSL_CONF_CTX_new.pod +++ b/doc/ssl/SSL_CONF_CTX_new.pod @@ -17,6 +17,7 @@ The function SSL_CONF_CTX_new() allocates and initialises an B structure for use with the SSL_CONF functions. The function SSL_CONF_CTX_free() frees up the context B. +If B is NULL nothing is done. =head1 RETURN VALUES diff --git a/doc/ssl/SSL_CTX_free.pod b/doc/ssl/SSL_CTX_free.pod index 51d8676968..f37617dcef 100644 --- a/doc/ssl/SSL_CTX_free.pod +++ b/doc/ssl/SSL_CTX_free.pod @@ -20,6 +20,8 @@ It also calls the free()ing procedures for indirectly affected items, if applicable: the session cache, the list of ciphers, the list of Client CAs, the certificates and keys. +If B is NULL nothing is done. + =head1 WARNINGS If a session-remove callback is set (SSL_CTX_sess_set_remove_cb()), this diff --git a/doc/ssl/SSL_SESSION_free.pod b/doc/ssl/SSL_SESSION_free.pod index 110ec73ab6..f30fe13c7d 100644 --- a/doc/ssl/SSL_SESSION_free.pod +++ b/doc/ssl/SSL_SESSION_free.pod @@ -15,6 +15,7 @@ SSL_SESSION_free - free an allocated SSL_SESSION structure SSL_SESSION_free() decrements the reference count of B and removes the B structure pointed to by B and frees up the allocated memory, if the reference count has reached 0. +If B is NULL nothing is done. =head1 NOTES diff --git a/doc/ssl/SSL_free.pod b/doc/ssl/SSL_free.pod index 13c1abd9ec..e3e6f56dc4 100644 --- a/doc/ssl/SSL_free.pod +++ b/doc/ssl/SSL_free.pod @@ -15,6 +15,7 @@ SSL_free - free an allocated SSL structure SSL_free() decrements the reference count of B, and removes the SSL structure pointed to by B and frees up the allocated memory if the reference count has reached 0. +If B is NULL nothing is done. =head1 NOTES diff --git a/ssl/bio_ssl.c b/ssl/bio_ssl.c index 0344b7e35b..7cf941d15b 100644 --- a/ssl/bio_ssl.c +++ b/ssl/bio_ssl.c @@ -125,7 +125,7 @@ static int ssl_free(BIO *a) if (bs->ssl != NULL) SSL_shutdown(bs->ssl); if (a->shutdown) { - if (a->init && (bs->ssl != NULL)) + if (a->init) SSL_free(bs->ssl); a->init = 0; a->flags = 0; @@ -416,8 +416,7 @@ static long ssl_ctrl(BIO *b, int cmd, long num, void *ptr) break; case BIO_CTRL_DUP: dbio = (BIO *)ptr; - if (((BIO_SSL *)dbio->ptr)->ssl != NULL) - SSL_free(((BIO_SSL *)dbio->ptr)->ssl); + SSL_free(((BIO_SSL *)dbio->ptr)->ssl); ((BIO_SSL *)dbio->ptr)->ssl = SSL_dup(ssl); ((BIO_SSL *)dbio->ptr)->renegotiate_count = ((BIO_SSL *)b->ptr)->renegotiate_count; diff --git a/ssl/s3_clnt.c b/ssl/s3_clnt.c index 6da125897d..404f7f9f8f 100644 --- a/ssl/s3_clnt.c +++ b/ssl/s3_clnt.c @@ -1215,8 +1215,7 @@ int ssl3_get_server_certificate(SSL *s) if (sc == NULL) goto err; - if (s->session->sess_cert) - ssl_sess_cert_free(s->session->sess_cert); + ssl_sess_cert_free(s->session->sess_cert); s->session->sess_cert = sc; sc->cert_chain = sk; diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c index abb3fd301f..cb7bd86e2a 100644 --- a/ssl/ssl_lib.c +++ b/ssl/ssl_lib.c @@ -393,8 +393,7 @@ SSL *SSL_new(SSL_CTX *ctx) return (s); err: - if (s != NULL) - SSL_free(s); + SSL_free(s); SSLerr(SSL_F_SSL_NEW, ERR_R_MALLOC_FAILURE); return (NULL); } @@ -2992,6 +2991,7 @@ int ssl_init_wbio_buffer(SSL *s, int push) void ssl_free_wbio_buffer(SSL *s) { + /* callers ensure s is never null */ if (s->bbio == NULL) return; diff --git a/ssl/ssl_sess.c b/ssl/ssl_sess.c index 9273eb6c48..24e5d259d4 100644 --- a/ssl/ssl_sess.c +++ b/ssl/ssl_sess.c @@ -292,10 +292,8 @@ int ssl_get_new_session(SSL *s, int session) else ss->timeout = s->session_ctx->session_timeout; - if (s->session != NULL) { - SSL_SESSION_free(s->session); - s->session = NULL; - } + SSL_SESSION_free(s->session); + s->session = NULL; if (session) { if (s->version == SSL3_VERSION) { @@ -578,8 +576,7 @@ int ssl_get_prev_session(SSL *s, unsigned char *session_id, int len, s->session_ctx->stats.sess_hit++; - if (s->session != NULL) - SSL_SESSION_free(s->session); + SSL_SESSION_free(s->session); s->session = ret; s->verify_result = s->session->verify_result; return 1; diff --git a/test/ssltest.c b/test/ssltest.c index c9f5b4da1d..6ad63427ad 100644 --- a/test/ssltest.c +++ b/test/ssltest.c @@ -1789,15 +1789,11 @@ int main(int argc, char *argv[]) SSL_free(c_ssl); end: - if (s_ctx != NULL) - SSL_CTX_free(s_ctx); - if (c_ctx != NULL) - SSL_CTX_free(c_ctx); - - if (s_cctx) - SSL_CONF_CTX_free(s_cctx); - if (c_cctx) - SSL_CONF_CTX_free(c_cctx); + SSL_CTX_free(s_ctx); + SSL_CTX_free(c_ctx); + + SSL_CONF_CTX_free(s_cctx); + SSL_CONF_CTX_free(c_cctx); sk_OPENSSL_STRING_free(conf_args); BIO_free(bio_stdout); -- 2.40.0