[Pkg-voip-commits] [pjproject] 08/14: Re #1969: Fix crash on using an already destroyed SSL

[Bernhard Schmidt]

This is an automated email from the git hooks/post-receive script.

berni pushed a commit to branch master
in repository pjproject.

commit 5a0ded59b572274fa1988d89afbbafeb2cbb34f1
Author: riza <riza at localhost>
Date:   Thu Oct 13 09:02:50 2016 +0000

    Re #1969: Fix crash on using an already destroyed SSL
    
     socket.
    
    Patch-Category: asterisk
---
 pjlib/src/pj/ssl_sock_ossl.c | 66 ++++++++++++++++++++++++++++----------------
 1 file changed, 42 insertions(+), 24 deletions(-)

diff --git a/pjlib/src/pj/ssl_sock_ossl.c b/pjlib/src/pj/ssl_sock_ossl.c
index 1e276ab..df69bd6 100644
--- a/pjlib/src/pj/ssl_sock_ossl.c
+++ b/pjlib/src/pj/ssl_sock_ossl.c
@@ -822,7 +822,10 @@ static void close_sockets(pj_ssl_sock_t *ssock)
     pj_lock_acquire(ssock->write_mutex);
     asock = ssock->asock;
     if (asock) {
-        ssock->asock = NULL;
+        // Don't set ssock->asock to NULL, as it may trigger assertion in
+        // send operation. This should be safe as active socket will simply
+        // return PJ_EINVALIDOP on any operation if it is already closed.
+        //ssock->asock = NULL;
         ssock->sock = PJ_INVALID_SOCKET;
     }
     sock = ssock->sock;
@@ -841,9 +844,9 @@ static void close_sockets(pj_ssl_sock_t *ssock)
 /* Reset SSL socket state */
 static void reset_ssl_sock_state(pj_ssl_sock_t *ssock)
 {
+    pj_lock_acquire(ssock->write_mutex);
     ssock->ssl_state = SSL_STATE_NULL;
-
-    destroy_ssl(ssock);
+    pj_lock_release(ssock->write_mutex);
 
     close_sockets(ssock);
 
@@ -1612,6 +1615,21 @@ static pj_status_t do_handshake(pj_ssl_sock_t *ssock)
     return PJ_EPENDING;
 }
 
+static void ssl_on_destroy(void *arg)
+{
+    pj_pool_t *pool = NULL;
+    pj_ssl_sock_t *ssock = (pj_ssl_sock_t*)arg;
+
+    destroy_ssl(ssock);
+
+    pj_lock_destroy(ssock->write_mutex);
+
+    pool = ssock->pool;
+    ssock->pool = NULL;
+    if (pool)
+	pj_pool_release(pool);
+}
+
 
 /*
  *******************************************************************
@@ -1830,7 +1848,7 @@ static pj_bool_t asock_on_accept_complete (pj_activesock_t *asock,
 
     /* Create new SSL socket instance */
     status = pj_ssl_sock_create(ssock_parent->pool,
-    				&ssock_parent->newsock_param, &ssock);
+				&ssock_parent->newsock_param, &ssock);
     if (status != PJ_SUCCESS)
 	goto on_return;
 
@@ -1906,12 +1924,10 @@ static pj_bool_t asock_on_accept_complete (pj_activesock_t *asock,
 	if (status != PJ_SUCCESS)
 	    goto on_return;
 
-	/* Temporarily add ref the group lock until active socket creation,
-	 * to make sure that group lock is destroyed if the active socket
-	 * creation fails.
-	 */
 	pj_grp_lock_add_ref(glock);
 	asock_cfg.grp_lock = ssock->param.grp_lock = glock;
+	pj_grp_lock_add_handler(ssock->param.grp_lock, ssock->pool, ssock,
+				ssl_on_destroy);
     }
 
     pj_bzero(&asock_cb, sizeof(asock_cb));
@@ -1927,11 +1943,6 @@ static pj_bool_t asock_on_accept_complete (pj_activesock_t *asock,
 				  ssock,
 				  &ssock->asock);
 
-    /* This will destroy the group lock if active socket creation fails */
-    if (asock_cfg.grp_lock) {
-	pj_grp_lock_dec_ref(asock_cfg.grp_lock);
-    }
-
     if (status != PJ_SUCCESS)
 	goto on_return;
 
@@ -2251,17 +2262,26 @@ PJ_DEF(pj_status_t) pj_ssl_sock_create (pj_pool_t *pool,
     /* Create secure socket mutex */
     status = pj_lock_create_recursive_mutex(pool, pool->obj_name,
 					    &ssock->write_mutex);
-    if (status != PJ_SUCCESS)
+    if (status != PJ_SUCCESS) {
+	pj_pool_release(pool);
 	return status;
+    }
 
     /* Init secure socket param */
     pj_ssl_sock_param_copy(pool, &ssock->param, param);
+
+    if (ssock->param.grp_lock) {
+	pj_grp_lock_add_ref(ssock->param.grp_lock);
+	pj_grp_lock_add_handler(ssock->param.grp_lock, pool, ssock,
+				ssl_on_destroy);
+    }
+
     ssock->param.read_buffer_size = ((ssock->param.read_buffer_size+7)>>3)<<3;
     if (!ssock->param.timer_heap) {
 	PJ_LOG(3,(ssock->pool->obj_name, "Warning: timer heap is not "
 		  "available. It is recommended to supply one to avoid "
-		  "a race condition if more than one worker threads "
-		  "are used."));
+	          "a race condition if more than one worker threads "
+	          "are used."));
     }
 
     /* Finally */
@@ -2277,8 +2297,6 @@ PJ_DEF(pj_status_t) pj_ssl_sock_create (pj_pool_t *pool,
  */
 PJ_DEF(pj_status_t) pj_ssl_sock_close(pj_ssl_sock_t *ssock)
 {
-    pj_pool_t *pool;
-
     PJ_ASSERT_RETURN(ssock, PJ_EINVAL);
 
     if (!ssock->pool)
@@ -2290,12 +2308,11 @@ PJ_DEF(pj_status_t) pj_ssl_sock_close(pj_ssl_sock_t *ssock)
     }
 
     reset_ssl_sock_state(ssock);
-    pj_lock_destroy(ssock->write_mutex);
-    
-    pool = ssock->pool;
-    ssock->pool = NULL;
-    if (pool)
-	pj_pool_release(pool);
+    if (ssock->param.grp_lock) {
+	pj_grp_lock_dec_ref(ssock->param.grp_lock);
+    } else {
+	ssl_on_destroy(ssock);
+    }
 
     return PJ_SUCCESS;
 }
@@ -2782,6 +2799,7 @@ pj_ssl_sock_start_accept2(pj_ssl_sock_t *ssock,
 
     /* Start accepting */
     pj_ssl_sock_param_copy(pool, &ssock->newsock_param, newsock_param);
+    ssock->newsock_param.grp_lock = NULL;
     status = pj_activesock_start_accept(ssock->asock, pool);
     if (status != PJ_SUCCESS)
 	goto on_error;

-- 
Alioth's /usr/local/bin/git-commit-notice on /srv/git.debian.org/git/pkg-voip/pjproject.git