openssl: fix possible SEGV on race between SSLSocket#stop and #connect

* ext/openssl/ossl_ssl.c (ossl_ssl_stop): Don't free the SSL struct
  here. Since some methods such as SSLSocket#connect releases GVL,
  there is a chance of use after free if we free the SSL from another
  thread. SSLSocket#stop was documented as "prepares it for another
  connection" so this is a slightly incompatible change. However when
  this sentence was added (r30090, Add toplevel documentation for
  OpenSSL, 2010-12-06), it didn't actually. The current behavior is
  from r40304 (Correct shutdown behavior w.r.t GC., 2013-04-15).
  [ruby-core:74978] [Bug #12292]

* ext/openssl/lib/openssl/ssl.rb (sysclose): Update doc.

* test/openssl/test_ssl.rb: Test this.

git-svn-id: svn+ssh://ci.ruby-lang.org/ruby/trunk@55100 b2dd03c8-39d4-4d8f-98ff-823fe69b080e

2 files changed, 8 insertions, 11 deletions

diff --git a/ext/openssl/lib/openssl/ssl.rb b/ext/openssl/lib/openssl/ssl.rb
index 57519f2c21..9893757011 100644
--- a/ext/openssl/lib/openssl/ssl.rb
+++ b/ext/openssl/lib/openssl/ssl.rb
@@ -290,7 +290,10 @@ module OpenSSL
       # call-seq:
       #    ssl.sysclose => nil
       #
-      # Shuts down the SSL connection and prepares it for another connection.
+      # Sends "close notify" to the peer and tries to shut down the SSL
+      # connection gracefully.
+      #
+      # If sync_close is set to +true+, the underlying IO is also closed.
       def sysclose
         return if closed?
         stop
diff --git a/ext/openssl/ossl_ssl.c b/ext/openssl/ossl_ssl.c
index 87df7f9f78..d5ea130489 100644
--- a/ext/openssl/ossl_ssl.c
+++ b/ext/openssl/ossl_ssl.c
@@ -1601,23 +1601,17 @@ ossl_ssl_write_nonblock(int argc, VALUE *argv, VALUE self)
  * call-seq:
  *    ssl.stop => nil
  *
- * Stops the SSL connection and prepares it for another connection.
+ * Sends "close notify" to the peer and tries to shut down the SSL connection
+ * gracefully.
  */
 static VALUE
 ossl_ssl_stop(VALUE self)
 {
     SSL *ssl;
 
-    /* ossl_ssl_data_get_struct() is not usable here because it may return
-     * from this function; */
-
-    GetSSL(self, ssl);
+    ossl_ssl_data_get_struct(self, ssl);
 
-    if (ssl) {
-	ossl_ssl_shutdown(ssl);
-	SSL_free(ssl);
-    }
-    DATA_PTR(self) = NULL;
+    ossl_ssl_shutdown(ssl);
 
     return Qnil;
 }