mirror of https://github.com/asterisk/asterisk
Addresses crashes when an attempt is made to operate on an SSL socket
after the socket has been closed.
ASTERISK-26477 #close
Change-Id: I421305b357558b4f9e690210dc0f4831ef4b3002
(cherry picked from commit 546ec4b038
)
changes/46/4146/1
parent
7c2bd702fd
commit
28cc8a9dff
@ -0,0 +1,164 @@
|
||||
From 9e67e0d5c3fdc747530a956038b374fca4748b76 Mon Sep 17 00:00:00 2001
|
||||
From: riza <riza@localhost>
|
||||
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
|
||||
|
Loading…
Reference in new issue