svn commit: r1851094 - in /tomcat/native/trunk: native/src/ssl.c xdocs/miscellaneous/changelog.xml

classic Classic list List threaded Threaded
4 messages Options
Reply | Threaded
Open this post in threaded view
|

svn commit: r1851094 - in /tomcat/native/trunk: native/src/ssl.c xdocs/miscellaneous/changelog.xml

markt
Author: markt
Date: Fri Jan 11 19:49:01 2019
New Revision: 1851094

URL: http://svn.apache.org/viewvc?rev=1851094&view=rev
Log:
Fix a per connection memory leak when using OpenSSL BIO. This is typically used when OpenSSL is providing the TLS support for NIO or NIO2.

Modified:
    tomcat/native/trunk/native/src/ssl.c
    tomcat/native/trunk/xdocs/miscellaneous/changelog.xml

Modified: tomcat/native/trunk/native/src/ssl.c
URL: http://svn.apache.org/viewvc/tomcat/native/trunk/native/src/ssl.c?rev=1851094&r1=1851093&r2=1851094&view=diff
==============================================================================
--- tomcat/native/trunk/native/src/ssl.c (original)
+++ tomcat/native/trunk/native/src/ssl.c Fri Jan 11 19:49:01 2019
@@ -1279,23 +1279,36 @@ TCN_IMPLEMENT_CALL(jlong /* SSL * */, SS
     tcn_ssl_ctxt_t *c = J2P(ctx, tcn_ssl_ctxt_t *);
     int *handshakeCount = malloc(sizeof(int));
     SSL *ssl;
+    apr_pool_t *p = NULL;
     tcn_ssl_conn_t *con;
 
     UNREFERENCED_STDARGS;
 
     TCN_ASSERT(ctx != 0);
+    
     ssl = SSL_new(c->ctx);
     if (ssl == NULL) {
         free(handshakeCount);
         tcn_ThrowException(e, "cannot create new ssl");
         return 0;
     }
-    if ((con = apr_pcalloc(c->pool, sizeof(tcn_ssl_conn_t))) == NULL) {
+    
+    apr_pool_create(&p, c->pool);
+    if (p == NULL) {
         free(handshakeCount);
+        SSL_free(ssl);
         tcn_ThrowAPRException(e, apr_get_os_error());
         return 0;
     }
-    con->pool = c->pool;
+    
+    if ((con = apr_pcalloc(p, sizeof(tcn_ssl_conn_t))) == NULL) {
+        free(handshakeCount);
+        SSL_free(ssl);
+        apr_pool_destroy(p);
+        tcn_ThrowAPRException(e, apr_get_os_error());
+        return 0;
+    }
+    con->pool = p;
     con->ctx  = c;
     con->ssl  = ssl;
     con->shutdown_type = c->shutdown_type;
@@ -1417,6 +1430,7 @@ TCN_IMPLEMENT_CALL(void, SSL, freeSSL)(T
                                        jlong ssl /* SSL * */) {
     SSL *ssl_ = J2P(ssl, SSL *);
     int *handshakeCount = SSL_get_app_data3(ssl_);
+    tcn_ssl_conn_t *con = SSL_get_app_data(ssl_);
 
     UNREFERENCED_STDARGS;
 
@@ -1424,6 +1438,7 @@ TCN_IMPLEMENT_CALL(void, SSL, freeSSL)(T
         free(handshakeCount);
     }
     SSL_free(ssl_);
+    apr_pool_destroy(con->pool);
 }
 
 /* Make a BIO pair (network and internal) for the provided SSL * and return the network BIO */

Modified: tomcat/native/trunk/xdocs/miscellaneous/changelog.xml
URL: http://svn.apache.org/viewvc/tomcat/native/trunk/xdocs/miscellaneous/changelog.xml?rev=1851094&r1=1851093&r2=1851094&view=diff
==============================================================================
--- tomcat/native/trunk/xdocs/miscellaneous/changelog.xml (original)
+++ tomcat/native/trunk/xdocs/miscellaneous/changelog.xml Fri Jan 11 19:49:01 2019
@@ -47,6 +47,10 @@
       Fix some minor memory leaks that could occur after error conditions during
       TLS connector initialisation. (markt)
     </fix>
+    <fix>
+      Fix a per connection memory leak when using OpenSSL BIO. This is typically
+      used when OpenSSL is providing the TLS support for NIO or NIO2. (markt)
+    </fix>
   </changelog>
 </section>
 <section name="Changes in 1.2.19">



---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r1851094 - in /tomcat/native/trunk: native/src/ssl.c xdocs/miscellaneous/changelog.xml

markt
On 11/01/2019 19:49, [hidden email] wrote:
> Author: markt
> Date: Fri Jan 11 19:49:01 2019
> New Revision: 1851094
>
> URL: http://svn.apache.org/viewvc?rev=1851094&view=rev
> Log:
> Fix a per connection memory leak when using OpenSSL BIO. This is typically used when OpenSSL is providing the TLS support for NIO or NIO2.

As usual, additional review of this fix welcome since this is me writing
C code.

I have tested it builds on Windows as well this time ;)

Mark


>
> Modified:
>     tomcat/native/trunk/native/src/ssl.c
>     tomcat/native/trunk/xdocs/miscellaneous/changelog.xml
>
> Modified: tomcat/native/trunk/native/src/ssl.c
> URL: http://svn.apache.org/viewvc/tomcat/native/trunk/native/src/ssl.c?rev=1851094&r1=1851093&r2=1851094&view=diff
> ==============================================================================
> --- tomcat/native/trunk/native/src/ssl.c (original)
> +++ tomcat/native/trunk/native/src/ssl.c Fri Jan 11 19:49:01 2019
> @@ -1279,23 +1279,36 @@ TCN_IMPLEMENT_CALL(jlong /* SSL * */, SS
>      tcn_ssl_ctxt_t *c = J2P(ctx, tcn_ssl_ctxt_t *);
>      int *handshakeCount = malloc(sizeof(int));
>      SSL *ssl;
> +    apr_pool_t *p = NULL;
>      tcn_ssl_conn_t *con;
>  
>      UNREFERENCED_STDARGS;
>  
>      TCN_ASSERT(ctx != 0);
> +    
>      ssl = SSL_new(c->ctx);
>      if (ssl == NULL) {
>          free(handshakeCount);
>          tcn_ThrowException(e, "cannot create new ssl");
>          return 0;
>      }
> -    if ((con = apr_pcalloc(c->pool, sizeof(tcn_ssl_conn_t))) == NULL) {
> +    
> +    apr_pool_create(&p, c->pool);
> +    if (p == NULL) {
>          free(handshakeCount);
> +        SSL_free(ssl);
>          tcn_ThrowAPRException(e, apr_get_os_error());
>          return 0;
>      }
> -    con->pool = c->pool;
> +    
> +    if ((con = apr_pcalloc(p, sizeof(tcn_ssl_conn_t))) == NULL) {
> +        free(handshakeCount);
> +        SSL_free(ssl);
> +        apr_pool_destroy(p);
> +        tcn_ThrowAPRException(e, apr_get_os_error());
> +        return 0;
> +    }
> +    con->pool = p;
>      con->ctx  = c;
>      con->ssl  = ssl;
>      con->shutdown_type = c->shutdown_type;
> @@ -1417,6 +1430,7 @@ TCN_IMPLEMENT_CALL(void, SSL, freeSSL)(T
>                                         jlong ssl /* SSL * */) {
>      SSL *ssl_ = J2P(ssl, SSL *);
>      int *handshakeCount = SSL_get_app_data3(ssl_);
> +    tcn_ssl_conn_t *con = SSL_get_app_data(ssl_);
>  
>      UNREFERENCED_STDARGS;
>  
> @@ -1424,6 +1438,7 @@ TCN_IMPLEMENT_CALL(void, SSL, freeSSL)(T
>          free(handshakeCount);
>      }
>      SSL_free(ssl_);
> +    apr_pool_destroy(con->pool);
>  }
>  
>  /* Make a BIO pair (network and internal) for the provided SSL * and return the network BIO */
>
> Modified: tomcat/native/trunk/xdocs/miscellaneous/changelog.xml
> URL: http://svn.apache.org/viewvc/tomcat/native/trunk/xdocs/miscellaneous/changelog.xml?rev=1851094&r1=1851093&r2=1851094&view=diff
> ==============================================================================
> --- tomcat/native/trunk/xdocs/miscellaneous/changelog.xml (original)
> +++ tomcat/native/trunk/xdocs/miscellaneous/changelog.xml Fri Jan 11 19:49:01 2019
> @@ -47,6 +47,10 @@
>        Fix some minor memory leaks that could occur after error conditions during
>        TLS connector initialisation. (markt)
>      </fix>
> +    <fix>
> +      Fix a per connection memory leak when using OpenSSL BIO. This is typically
> +      used when OpenSSL is providing the TLS support for NIO or NIO2. (markt)
> +    </fix>
>    </changelog>
>  </section>
>  <section name="Changes in 1.2.19">
>
>
>
> ---------------------------------------------------------------------
> To unsubscribe, e-mail: [hidden email]
> For additional commands, e-mail: [hidden email]
>


---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r1851094 - in /tomcat/native/trunk: native/src/ssl.c xdocs/miscellaneous/changelog.xml

markt
On 11/01/2019 20:01, Mark Thomas wrote:

> On 11/01/2019 19:49, [hidden email] wrote:
>> Author: markt
>> Date: Fri Jan 11 19:49:01 2019
>> New Revision: 1851094
>>
>> URL: http://svn.apache.org/viewvc?rev=1851094&view=rev
>> Log:
>> Fix a per connection memory leak when using OpenSSL BIO. This is typically used when OpenSSL is providing the TLS support for NIO or NIO2.
>
> As usual, additional review of this fix welcome since this is me writing
> C code.
>
> I have tested it builds on Windows as well this time ;)

I've seen a APR/native crash on shutdown since applying this fix
although it doesn't appear consistently. I'm looking into it.

Mark

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

Re: svn commit: r1851094 - in /tomcat/native/trunk: native/src/ssl.c xdocs/miscellaneous/changelog.xml

markt
On 14/01/2019 14:55, Mark Thomas wrote:

> On 11/01/2019 20:01, Mark Thomas wrote:
>> On 11/01/2019 19:49, [hidden email] wrote:
>>> Author: markt
>>> Date: Fri Jan 11 19:49:01 2019
>>> New Revision: 1851094
>>>
>>> URL: http://svn.apache.org/viewvc?rev=1851094&view=rev
>>> Log:
>>> Fix a per connection memory leak when using OpenSSL BIO. This is typically used when OpenSSL is providing the TLS support for NIO or NIO2.
>>
>> As usual, additional review of this fix welcome since this is me writing
>> C code.
>>
>> I have tested it builds on Windows as well this time ;)
>
> I've seen a APR/native crash on shutdown since applying this fix
> although it doesn't appear consistently. I'm looking into it.

I can't repeat this.

I'm planning on tagging 1.2.20 shortly.

Mark

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]