[Asterisk-code-review] Revert "pjproject bundled: Add patch to address SSL crash" (asterisk[certified/13.8])
Joshua Colp
asteriskteam at digium.com
Tue Oct 18 16:50:29 CDT 2016
Hello George Joseph,
I'd like you to do a code review. Please visit
https://gerrit.asterisk.org/4147
to review the following change.
Change subject: Revert "pjproject_bundled: Add patch to address SSL crash"
......................................................................
Revert "pjproject_bundled: Add patch to address SSL crash"
This reverts commit 28cc8a9dff2fb9210726cfa6274ae683fbfa4a01.
Change-Id: I777cf8173f7a88273090bed72bfe57fb0e72b84f
---
D third-party/pjproject/patches/0005-Re-1969-Fix-crash-on-using-an-already-destroyed-SSL-.patch
1 file changed, 0 insertions(+), 164 deletions(-)
git pull ssh://gerrit.asterisk.org:29418/asterisk refs/changes/47/4147/1
diff --git a/third-party/pjproject/patches/0005-Re-1969-Fix-crash-on-using-an-already-destroyed-SSL-.patch b/third-party/pjproject/patches/0005-Re-1969-Fix-crash-on-using-an-already-destroyed-SSL-.patch
deleted file mode 100644
index 551e61a..0000000
--- a/third-party/pjproject/patches/0005-Re-1969-Fix-crash-on-using-an-already-destroyed-SSL-.patch
+++ /dev/null
@@ -1,164 +0,0 @@
-From 9e67e0d5c3fdc747530a956038b374fca4748b76 Mon Sep 17 00:00:00 2001
-From: riza <riza at 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
-
--
To view, visit https://gerrit.asterisk.org/4147
To unsubscribe, visit https://gerrit.asterisk.org/settings
Gerrit-MessageType: newchange
Gerrit-Change-Id: I777cf8173f7a88273090bed72bfe57fb0e72b84f
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: certified/13.8
Gerrit-Owner: Joshua Colp <jcolp at digium.com>
Gerrit-Reviewer: George Joseph <gjoseph at digium.com>
More information about the asterisk-code-review
mailing list