changeset 7955:0601a4e793bf stable-1.20

SSL: ngx_ssl_shutdown() rework. Instead of calling SSL_free() with each return point, introduced a single place where cleanup happens. As a positive side effect, this fixes two potential memory leaks on ngx_handle_read_event() and ngx_handle_write_event() errors where there were no SSL_free() calls (though unlikely practical, as errors there are only expected to happen due to bugs or kernel issues).
author Maxim Dounin <mdounin@mdounin.ru>
date Tue, 01 Jun 2021 17:37:49 +0300
parents e9a98fb95c48
children ae70fcb8ac93
files src/event/ngx_event_openssl.c
diffstat 1 files changed, 22 insertions(+), 23 deletions(-) [+]
line wrap: on
line diff
--- a/src/event/ngx_event_openssl.c
+++ b/src/event/ngx_event_openssl.c
@@ -2896,9 +2896,12 @@ ngx_int_t
 ngx_ssl_shutdown(ngx_connection_t *c)
 {
     int         n, sslerr, mode;
+    ngx_int_t   rc;
     ngx_err_t   err;
     ngx_uint_t  tries;
 
+    rc = NGX_OK;
+
     ngx_ssl_ocsp_cleanup(c);
 
     if (SSL_in_init(c->ssl->connection)) {
@@ -2908,11 +2911,7 @@ ngx_ssl_shutdown(ngx_connection_t *c)
          * Avoid calling SSL_shutdown() if handshake wasn't completed.
          */
 
-        SSL_free(c->ssl->connection);
-        c->ssl = NULL;
-        c->recv = ngx_recv;
-
-        return NGX_OK;
+        goto done;
     }
 
     if (c->timedout || c->error || c->buffered) {
@@ -2954,11 +2953,7 @@ ngx_ssl_shutdown(ngx_connection_t *c)
         ngx_log_debug1(NGX_LOG_DEBUG_EVENT, c->log, 0, "SSL_shutdown: %d", n);
 
         if (n == 1) {
-            SSL_free(c->ssl->connection);
-            c->ssl = NULL;
-            c->recv = ngx_recv;
-
-            return NGX_OK;
+            goto done;
         }
 
         if (n == 0 && tries-- > 1) {
@@ -2984,11 +2979,11 @@ ngx_ssl_shutdown(ngx_connection_t *c)
             }
 
             if (ngx_handle_read_event(c->read, 0) != NGX_OK) {
-                return NGX_ERROR;
+                goto failed;
             }
 
             if (ngx_handle_write_event(c->write, 0) != NGX_OK) {
-                return NGX_ERROR;
+                goto failed;
             }
 
             ngx_add_timer(c->read, 3000);
@@ -2997,23 +2992,27 @@ ngx_ssl_shutdown(ngx_connection_t *c)
         }
 
         if (sslerr == SSL_ERROR_ZERO_RETURN || ERR_peek_error() == 0) {
-            SSL_free(c->ssl->connection);
-            c->ssl = NULL;
-            c->recv = ngx_recv;
-
-            return NGX_OK;
+            goto done;
         }
 
         err = (sslerr == SSL_ERROR_SYSCALL) ? ngx_errno : 0;
 
         ngx_ssl_connection_error(c, sslerr, err, "SSL_shutdown() failed");
 
-        SSL_free(c->ssl->connection);
-        c->ssl = NULL;
-        c->recv = ngx_recv;
-
-        return NGX_ERROR;
-    }
+        break;
+    }
+
+failed:
+
+    rc = NGX_ERROR;
+
+done:
+
+    SSL_free(c->ssl->connection);
+    c->ssl = NULL;
+    c->recv = ngx_recv;
+
+    return rc;
 }