kris/boringssl
1
0
Fork 0
Code Issues Pull Requests Releases Wiki Activity

Have fun with lock scopers.

Browse Source 
Change-Id: I2697349024769545c2c37173e6ed68640b7d3b78
Reviewed-on: https://boringssl-review.googlesource.com/20805
Reviewed-by: Adam Langley <agl@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
CQ-Verified: CQ bot account: commit-bot@chromium.org <commit-bot@chromium.org>
This commit is contained in:
David Benjamin 2017-09-27 17:28:10 -04:00 committed by CQ bot account: commit-bot@chromium.org
parent 9eaa3bd55d
commit 21fa684236
2 changed files with 22 additions and 28 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

28
ssl/ssl_session.cc
View File

@ -672,14 +672,13 @@ static enum ssl_hs_wait_t ssl_lookup_session(
data.session_id_length = session_id_len;
OPENSSL_memcpy(data.session_id, session_id, session_id_len);
CRYPTO_MUTEX_lock_read(&ssl->session_ctx->lock);
MutexReadLock lock(&ssl->session_ctx->lock);
session.reset(lh_SSL_SESSION_retrieve(ssl->session_ctx->sessions, &data));
if (session) {
// |lh_SSL_SESSION_retrieve| returns a non-owning pointer.
SSL_SESSION_up_ref(session.get());
}
// TODO(davidben): This should probably move it to the front of the list.
CRYPTO_MUTEX_unlock_read(&ssl->session_ctx->lock);
}
// Fall back to the external cache, if it exists.
@ -1014,27 +1013,30 @@ int SSL_CTX_add_session(SSL_CTX *ctx, SSL_SESSION *session) {
// Although |session| is inserted into two structures (a doubly-linked list
// and the hash table), |ctx| only takes one reference.
SSL_SESSION_up_ref(session);
UniquePtr<SSL_SESSION> owned_session(session);
SSL_SESSION *old_session;
CRYPTO_MUTEX_lock_write(&ctx->lock);
MutexWriteLock lock(&ctx->lock);
if (!lh_SSL_SESSION_insert(ctx->sessions, &old_session, session)) {
CRYPTO_MUTEX_unlock_write(&ctx->lock);
SSL_SESSION_free(session);
return 0;
}
// |ctx->sessions| took ownership of |session| and gave us back a reference to
// |old_session|. (|old_session| may be the same as |session|, in which case
// we traded identical references with |ctx->sessions|.)
owned_session.release();
owned_session.reset(old_session);
if (old_session != NULL) {
if (old_session == session) {
// |session| was already in the cache.
CRYPTO_MUTEX_unlock_write(&ctx->lock);
SSL_SESSION_free(old_session);
// |session| was already in the cache. There are no linked list pointers
// to update.
return 0;
}
// There was a session ID collision. |old_session| must be removed from
// the linked list and released.
// There was a session ID collision. |old_session| was replaced with
// |session| in the hash table, so |old_session| must be removed from the
// linked list to match.
SSL_SESSION_list_remove(ctx, old_session);
SSL_SESSION_free(old_session);
}
SSL_SESSION_list_add(ctx, session);
@ -1049,7 +1051,6 @@ int SSL_CTX_add_session(SSL_CTX *ctx, SSL_SESSION *session) {
}
}
CRYPTO_MUTEX_unlock_write(&ctx->lock);
return 1;
}
@ -1128,9 +1129,8 @@ void SSL_CTX_flush_sessions(SSL_CTX *ctx, uint64_t time) {
return;
}
tp.time = time;
CRYPTO_MUTEX_lock_write(&ctx->lock);
MutexWriteLock lock(&ctx->lock);
lh_SSL_SESSION_doall_arg(tp.cache, timeout_doall_arg, &tp);
CRYPTO_MUTEX_unlock_write(&ctx->lock);
}
void SSL_CTX_sess_set_new_cb(SSL_CTX *ctx,

22
ssl/ssl_x509.cc
View File

@ -835,10 +835,8 @@ X509 *SSL_get_certificate(const SSL *ssl) {
X509 *SSL_CTX_get0_certificate(const SSL_CTX *ctx) {
check_ssl_ctx_x509_method(ctx);
CRYPTO_MUTEX_lock_write((CRYPTO_MUTEX *) &ctx->lock);
X509 *ret = ssl_cert_get0_leaf(ctx->cert);
CRYPTO_MUTEX_unlock_write((CRYPTO_MUTEX *) &ctx->lock);
return ret;
MutexWriteLock lock(const_cast<CRYPTO_MUTEX*>(&ctx->lock));
return ssl_cert_get0_leaf(ctx->cert);
}
static int ssl_cert_set0_chain(CERT *cert, STACK_OF(X509) *chain) {
@ -994,11 +992,8 @@ static int ssl_cert_cache_chain_certs(CERT *cert) {
int SSL_CTX_get0_chain_certs(const SSL_CTX *ctx, STACK_OF(X509) **out_chain) {
check_ssl_ctx_x509_method(ctx);
CRYPTO_MUTEX_lock_write((CRYPTO_MUTEX *) &ctx->lock);
const int ret = ssl_cert_cache_chain_certs(ctx->cert);
CRYPTO_MUTEX_unlock_write((CRYPTO_MUTEX *) &ctx->lock);
if (!ret) {
MutexWriteLock lock(const_cast<CRYPTO_MUTEX*>(&ctx->lock));
if (!ssl_cert_cache_chain_certs(ctx->cert)) {
*out_chain = NULL;
return 0;
}
@ -1165,11 +1160,10 @@ STACK_OF(X509_NAME) *SSL_CTX_get_client_CA_list(const SSL_CTX *ctx) {
check_ssl_ctx_x509_method(ctx);
// This is a logically const operation that may be called on multiple threads,
// so it needs to lock around updating |cached_x509_client_CA|.
CRYPTO_MUTEX_lock_write((CRYPTO_MUTEX *) &ctx->lock);
STACK_OF(X509_NAME) *ret = buffer_names_to_x509(
ctx->client_CA, (STACK_OF(X509_NAME) **)&ctx->cached_x509_client_CA);
CRYPTO_MUTEX_unlock_write((CRYPTO_MUTEX *) &ctx->lock);
return ret;
MutexWriteLock lock(const_cast<CRYPTO_MUTEX *>(&ctx->lock));
return buffer_names_to_x509(
ctx->client_CA,
const_cast<STACK_OF(X509_NAME) **>(&ctx->cached_x509_client_CA));
}
static int add_client_CA(STACK_OF(CRYPTO_BUFFER) **names, X509 *x509,