Implement SSL_get1_session with SSL_SESSION_up_ref.
It doesn't appear that logic (added in upstream's
b7cfcfb7f8e17c17f457b3384010eb027f3aad72) is protecting against anything. On
the contrary, it prohibits implementing CRYPTO_add with real atomic operations!
There's no guarantee that those operations will interact with the locked
implementation.
https://www.mail-archive.com/openssl-users@openssl.org/msg63176.html
As long as ssl->session points to the same session, we know the session won't
be freed. There is no lock protecting, say, SSL_set_session, but a single SSL*
does not appear to be safe to use across threads. If this were to be supported,
both should be guarded by CRYPTO_LOCK_SSL (which is barely used).
CRYPTO_LOCK_SSL_SESSION isn't sufficient anyway; it could sample while
SSL_set_session is busy swapping out the now freed old session with the new.
Change-Id: I54623d0690c55c2c86720406ceff545e2e5f2f8f
Reviewed-on: https://boringssl-review.googlesource.com/3345
Reviewed-by: Adam Langley <agl@google.com>
diff --git a/ssl/ssl_sess.c b/ssl/ssl_sess.c
index 97dc10f..c6913fc 100644
--- a/ssl/ssl_sess.c
+++ b/ssl/ssl_sess.c
@@ -164,18 +164,7 @@
SSL_SESSION *SSL_get1_session(SSL *ssl) {
/* variant of SSL_get_session: caller really gets something */
- SSL_SESSION *sess;
- /* Need to lock this all up rather than just use CRYPTO_add so that
- * somebody doesn't free ssl->session between when we check it's
- * non-null and when we up the reference count. */
- CRYPTO_w_lock(CRYPTO_LOCK_SSL_SESSION);
- sess = ssl->session;
- if (sess) {
- sess->references++;
- }
- CRYPTO_w_unlock(CRYPTO_LOCK_SSL_SESSION);
-
- return sess;
+ return SSL_SESSION_up_ref(ssl->session);
}
int SSL_SESSION_get_ex_new_index(long argl, void *argp, CRYPTO_EX_new *new_func,
