summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authornagachika <nagachika@b2dd03c8-39d4-4d8f-98ff-823fe69b080e>2016-08-11 17:58:25 +0000
committernagachika <nagachika@b2dd03c8-39d4-4d8f-98ff-823fe69b080e>2016-08-11 17:58:25 +0000
commit88d76cfdcd19bab721bfefadaa4f1c0de4e3c0ff (patch)
treeade2fbcdbbb0f82734ba1392a81e6b35e2c326a9
parenta3600fc3771818028f591969c2d3287fcecfa13c (diff)
merge revision(s) 55100: [Backport #12292]
* 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/branches/ruby_2_3@55866 b2dd03c8-39d4-4d8f-98ff-823fe69b080e
-rw-r--r--ChangeLog16
-rw-r--r--ext/openssl/lib/openssl/ssl.rb5
-rw-r--r--ext/openssl/ossl_ssl.c14
-rw-r--r--test/openssl/test_ssl.rb22
-rw-r--r--version.h6
5 files changed, 49 insertions, 14 deletions
diff --git a/ChangeLog b/ChangeLog
index 44563a2fd4..aa83e4b1af 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,19 @@
+Fri Aug 12 02:46:37 2016 Kazuki Yamaguchi <k@rhe.jp>
+
+ * 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.
+
Thu Aug 11 01:30:29 2016 Marcus Stollsteimer <sto.mar@web.de>
* ext/json/lib/*.rb: Removed some comments. Because these are unnecessary
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 0e01d7c0b1..b30c2cbf07 100644
--- a/ext/openssl/ossl_ssl.c
+++ b/ext/openssl/ossl_ssl.c
@@ -1597,23 +1597,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;
}
diff --git a/test/openssl/test_ssl.rb b/test/openssl/test_ssl.rb
index d5e3547ba6..22478475ad 100644
--- a/test/openssl/test_ssl.rb
+++ b/test/openssl/test_ssl.rb
@@ -1169,6 +1169,28 @@ end
}
end
+ def test_close_and_socket_close_while_connecting
+ # test it doesn't cause a segmentation fault
+ ctx = OpenSSL::SSL::SSLContext.new
+ ctx.ciphers = "aNULL"
+
+ sock1, sock2 = socketpair
+ ssl1 = OpenSSL::SSL::SSLSocket.new(sock1, ctx)
+ ssl2 = OpenSSL::SSL::SSLSocket.new(sock2, ctx)
+
+ t = Thread.new { ssl1.connect }
+ ssl2.accept
+
+ ssl1.close
+ sock1.close
+ t.value rescue nil
+ ensure
+ ssl1.close if ssl1
+ ssl2.close if ssl2
+ sock1.close if sock1
+ sock2.close if sock2
+ end
+
private
def start_server_version(version, ctx_proc=nil, server_proc=nil, &blk)
diff --git a/version.h b/version.h
index bcd0f51145..7d8003850f 100644
--- a/version.h
+++ b/version.h
@@ -1,10 +1,10 @@
#define RUBY_VERSION "2.3.2"
-#define RUBY_RELEASE_DATE "2016-08-11"
-#define RUBY_PATCHLEVEL 146
+#define RUBY_RELEASE_DATE "2016-08-12"
+#define RUBY_PATCHLEVEL 147
#define RUBY_RELEASE_YEAR 2016
#define RUBY_RELEASE_MONTH 8
-#define RUBY_RELEASE_DAY 11
+#define RUBY_RELEASE_DAY 12
#include "ruby/version.h"