[前][次][番号順一覧][スレッド一覧]

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/

[前][次][番号順一覧][スレッド一覧]