1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
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
|
