summaryrefslogtreecommitdiff
path: root/ext
diff options
context:
space:
mode:
authorrhe <rhe@b2dd03c8-39d4-4d8f-98ff-823fe69b080e>2016-05-21 07:25:00 +0000
committerrhe <rhe@b2dd03c8-39d4-4d8f-98ff-823fe69b080e>2016-05-21 07:25:00 +0000
commit118ee2a734152c35ebb6e963d4052d7152ab3ba0 (patch)
treedb63612290d1877043cf96f49c4969d8b088aa77 /ext
parent77d1e6052f24787fc5fa2efe0c017c7e50c89de4 (diff)
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
Diffstat (limited to 'ext')
-rw-r--r--ext/openssl/lib/openssl/ssl.rb5
-rw-r--r--ext/openssl/ossl_ssl.c14
2 files changed, 8 insertions, 11 deletions
diff --git a/ext/openssl/lib/openssl/ssl.rb b/ext/openssl/lib/openssl/ssl.rb
index 57519f2..9893757 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 87df7f9..d5ea130 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;
}