Fix a memory leak in 1.1.0 related to initial_ctx
authorNeel Goyal <ngoyal@fb.com>
Thu, 15 Jun 2017 14:51:57 +0000 (07:51 -0700)
committerFacebook Github Bot <facebook-github-bot@users.noreply.github.com>
Thu, 15 Jun 2017 15:05:41 +0000 (08:05 -0700)
Summary: We would always up_ref the ctx before setting it as the initial_ctx.  This causes a leak in 1.1.0 since the initial_ctx isn't set in this version of OpenSSL.  We'll move the up_ref for the initial_ctx into the OpenSSLUtils helper.

Reviewed By: anirudhvr

Differential Revision: D5227823

fbshipit-source-id: b4490b317bd4dc8752a8d7e244fd153100a52aa6

folly/io/async/AsyncSSLSocket.cpp
folly/io/async/ssl/OpenSSLUtils.cpp

index 1379bf90bc91fe37c0285cda33e7522882b85871..84354730cc7b3a9c09f490e59ff6a8ae972497e0 100644 (file)
@@ -503,9 +503,6 @@ void AsyncSSLSocket::attachSSLContext(
   // In order to call attachSSLContext, detachSSLContext must have been
   // previously called.
   // We need to update the initial_ctx if necessary
-  auto sslCtx = ctx->getSSLCtx();
-  SSL_CTX_up_ref(sslCtx);
-
   // The 'initial_ctx' inside an SSL* points to the context that it was created
   // with, which is also where session callbacks and servername callbacks
   // happen.
@@ -514,6 +511,7 @@ void AsyncSSLSocket::attachSSLContext(
   // NOTE: this will only work if we have access to ssl_ internals, so it may
   // not work on
   // OpenSSL version >= 1.1.0
+  auto sslCtx = ctx->getSSLCtx();
   OpenSSLUtils::setSSLInitialCtx(ssl_, sslCtx);
   // Detach sets the socket's context to the dummy context. Thus we must acquire
   // this lock.
index 80d90ec04c92e409dbd618cf5618f0b4cf711bc0..91e114a03024e30569d2bf59c54da54f73456615 100644 (file)
@@ -200,6 +200,9 @@ void OpenSSLUtils::setSSLInitialCtx(SSL* ssl, SSL_CTX* ctx) {
   (void)ctx;
 #if !FOLLY_OPENSSL_IS_110 && !defined(OPENSSL_NO_TLSEXT)
   if (ssl) {
+    if (ctx) {
+      SSL_CTX_up_ref(ctx);
+    }
     ssl->initial_ctx = ctx;
   }
 #endif