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>
This commit is contained in:
parent
3363984d0d
commit
23a8ca1f10
@ -164,18 +164,7 @@ SSL_SESSION *SSL_get_session(const SSL *ssl)
|
||||
|
||||
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,
|
||||
|
Loading…
Reference in New Issue
Block a user