ruby-changes:43793
From: nagachika <ko1@a...>
Date: Fri, 12 Aug 2016 02:58:30 +0900 (JST)
Subject: [ruby-changes:43793] nagachika:r55866 (ruby_2_3): merge revision(s) 55100: [Backport #12292]
nagachika 2016-08-12 02:58:25 +0900 (Fri, 12 Aug 2016) New Revision: 55866 https://svn.ruby-lang.org/cgi-bin/viewvc.cgi?view=revision&revision=55866 Log: 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. Modified directories: branches/ruby_2_3/ Modified files: branches/ruby_2_3/ChangeLog branches/ruby_2_3/ext/openssl/lib/openssl/ssl.rb branches/ruby_2_3/ext/openssl/ossl_ssl.c branches/ruby_2_3/test/openssl/test_ssl.rb branches/ruby_2_3/version.h Index: ruby_2_3/ChangeLog =================================================================== --- ruby_2_3/ChangeLog (revision 55865) +++ ruby_2_3/ChangeLog (revision 55866) @@ -1,3 +1,19 @@ https://github.com/ruby/ruby/blob/trunk/ruby_2_3/ChangeLog#L1 +Fri Aug 12 02:46:37 2016 Kazuki Yamaguchi <k@r...> + + * 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@w...> * ext/json/lib/*.rb: Removed some comments. Because these are unnecessary Index: ruby_2_3/test/openssl/test_ssl.rb =================================================================== --- ruby_2_3/test/openssl/test_ssl.rb (revision 55865) +++ ruby_2_3/test/openssl/test_ssl.rb (revision 55866) @@ -1169,6 +1169,28 @@ end https://github.com/ruby/ruby/blob/trunk/ruby_2_3/test/openssl/test_ssl.rb#L1169 } 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) Index: ruby_2_3/version.h =================================================================== --- ruby_2_3/version.h (revision 55865) +++ ruby_2_3/version.h (revision 55866) @@ -1,10 +1,10 @@ https://github.com/ruby/ruby/blob/trunk/ruby_2_3/version.h#L1 #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" Index: ruby_2_3/ext/openssl/lib/openssl/ssl.rb =================================================================== --- ruby_2_3/ext/openssl/lib/openssl/ssl.rb (revision 55865) +++ ruby_2_3/ext/openssl/lib/openssl/ssl.rb (revision 55866) @@ -290,7 +290,10 @@ module OpenSSL https://github.com/ruby/ruby/blob/trunk/ruby_2_3/ext/openssl/lib/openssl/ssl.rb#L290 # 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 Index: ruby_2_3/ext/openssl/ossl_ssl.c =================================================================== --- ruby_2_3/ext/openssl/ossl_ssl.c (revision 55865) +++ ruby_2_3/ext/openssl/ossl_ssl.c (revision 55866) @@ -1597,23 +1597,17 @@ ossl_ssl_write_nonblock(int argc, VALUE https://github.com/ruby/ruby/blob/trunk/ruby_2_3/ext/openssl/ossl_ssl.c#L1597 * 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; */ + ossl_ssl_data_get_struct(self, ssl); - GetSSL(self, ssl); - - if (ssl) { - ossl_ssl_shutdown(ssl); - SSL_free(ssl); - } - DATA_PTR(self) = NULL; + ossl_ssl_shutdown(ssl); return Qnil; } Property changes on: ruby_2_3 ___________________________________________________________________ Modified: svn:mergeinfo Merged /trunk:r55100 -- ML: ruby-changes@q... Info: http://www.atdot.net/~ko1/quickml/