From 9e67e0d5c3fdc747530a956038b374fca4748b76 Mon Sep 17 00:00:00 2001 From: riza Date: Thu, 13 Oct 2016 09:02:50 +0000 Subject: [PATCH 1/4] Re #1969: Fix crash on using an already destroyed SSL socket. --- 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 fa0db2d..ceab67a 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; -- 2.7.4