Changeset 5459 for pjproject/trunk/pjlib/src/pj/ssl_sock_ossl.c

Timestamp:

Oct 13, 2016 9:02:50 AM (8 years ago)

Author:

riza

Message:

Re #1969: Fix crash on using an already destroyed SSL socket.

File:

• pjproject/trunk/pjlib/src/pj/ssl_sock_ossl.c (modified) (10 diffs)

pjproject/trunk/pjlib/src/pj/ssl_sock_ossl.c

@@ -823 +823 @@
     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;
     }
@@ -842 +845 @@
 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);
@@ -1613 +1616 @@
 }
 
+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);
+}
+
 
 /*
@@ -1831 +1849 @@
     /* 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;
@@ -1907 +1925 @@
             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);
     }
 
@@ -1927 +1943 @@
                                   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)
@@ -2252 +2263 @@
     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."));
     }
 
@@ -2278 +2298 @@
 PJ_DEF(pj_status_t) pj_ssl_sock_close(pj_ssl_sock_t *ssock)
 {
-    pj_pool_t *pool;
-
     PJ_ASSERT_RETURN(ssock, PJ_EINVAL);
 
@@ -2291 +2309 @@
 
     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;
@@ -2783 +2800 @@
     /* 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)