Skip to content

Commit f543ad1

Browse files
ylavicylavic
committedDec 3, 2020
apr_thread: use unmanaged pools for detached threads.
A detached thread is by definition out of control, unjoinable, unmanaged, and it can terminate/exit after its parent pool is detroyed. To avoid use-after-free in this case, let's use an unmanaged pool for detached threads, either by creating an unmanaged pool from the start if the thread is created detached, or by "unmanaging" the pool if the thread is detached later with apr_thread_detach(). To "umanage" the pool, provide a new internal helper, apr__pool_unmanage() which takes care of removing the pool from its parent's list. git-svn-id: https://svn.apache.org/repos/asf/apr/apr/trunk@1884078 13f79535-47bb-0310-9956-ffa450edef68
·
1.7.1-test
1 parent ee6a4a8 commit f543ad1

File tree

6 files changed

+203
-11
lines changed

6 files changed

+203
-11
lines changed
 

‎memory/unix/apr_pools.c

+38
Original file line numberDiff line numberDiff line change
@@ -2341,6 +2341,44 @@ APR_DECLARE(void) apr_pool_lock(apr_pool_t *pool, int flag)
23412341

23422342
#endif /* !APR_POOL_DEBUG */
23432343

2344+
/* For APR internal use only (for now).
2345+
* Detach the pool from its/any parent (i.e. un-manage).
2346+
*/
2347+
apr_status_t apr__pool_unmanage(apr_pool_t *pool)
2348+
{
2349+
apr_pool_t *parent = pool->parent;
2350+
2351+
if (!parent) {
2352+
return APR_NOTFOUND;
2353+
}
2354+
2355+
#if APR_POOL_DEBUG
2356+
if (pool->allocator && pool->allocator == parent->allocator) {
2357+
return APR_EINVAL;
2358+
}
2359+
apr_thread_mutex_lock(parent->mutex);
2360+
#else
2361+
if (pool->allocator == parent->allocator) {
2362+
return APR_EINVAL;
2363+
}
2364+
allocator_lock(parent->allocator);
2365+
#endif
2366+
2367+
/* Remove the pool from the parent's children */
2368+
if ((*pool->ref = pool->sibling) != NULL) {
2369+
pool->sibling->ref = pool->ref;
2370+
}
2371+
pool->parent = NULL;
2372+
2373+
#if APR_POOL_DEBUG
2374+
apr_thread_mutex_unlock(parent->mutex);
2375+
#else
2376+
allocator_unlock(parent->allocator);
2377+
#endif
2378+
2379+
return APR_SUCCESS;
2380+
}
2381+
23442382
#ifdef NETWARE
23452383
void netware_pool_proc_cleanup ()
23462384
{

‎threadproc/beos/thread.c

+33-5
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,9 @@
1717
#include "apr_arch_threadproc.h"
1818
#include "apr_portable.h"
1919

20+
/* Internal (from apr_pools.c) */
21+
extern apr_status_t apr__pool_unmanage(apr_pool_t *pool);
22+
2023
APR_DECLARE(apr_status_t) apr_threadattr_create(apr_threadattr_t **new, apr_pool_t *pool)
2124
{
2225
(*new) = (apr_threadattr_t *)apr_palloc(pool,
@@ -90,19 +93,42 @@ APR_DECLARE(apr_status_t) apr_thread_create(apr_thread_t **new, apr_threadattr_t
9093
(*new)->data = data;
9194
(*new)->func = func;
9295
(*new)->exitval = -1;
96+
9397
(*new)->detached = (attr && apr_threadattr_detach_get(attr) == APR_DETACH);
98+
if ((*new)->detached) {
99+
stat = apr_pool_create_unmanaged_ex(&(*new)->pool,
100+
apr_pool_abort_get(pool),
101+
NULL);
102+
}
103+
else {
104+
/* The thread can be apr_thread_detach()ed later, so the pool needs
105+
* its own allocator to not depend on the parent pool which could be
106+
* destroyed before the thread exits. The allocator needs no mutex
107+
* obviously since the pool should not be used nor create children
108+
* pools outside the thread.
109+
*/
110+
apr_allocator_t *allocator;
111+
if (apr_allocator_create(&allocator) != APR_SUCCESS) {
112+
return APR_ENOMEM;
113+
}
114+
stat = apr_pool_create_ex(&(*new)->pool, pool, NULL, allocator);
115+
if (stat == APR_SUCCESS) {
116+
apr_allocator_owner_set(allocator, (*new)->pool);
117+
}
118+
else {
119+
apr_allocator_destroy(allocator);
120+
}
121+
}
122+
if (stat != APR_SUCCESS) {
123+
return stat;
124+
}
94125

95126
/* First we create the new thread...*/
96127
if (attr)
97128
temp = attr->attr;
98129
else
99130
temp = B_NORMAL_PRIORITY;
100131

101-
stat = apr_pool_create(&(*new)->pool, pool);
102-
if (stat != APR_SUCCESS) {
103-
return stat;
104-
}
105-
106132
(*new)->td = spawn_thread((thread_func)dummy_worker,
107133
"apr thread",
108134
temp,
@@ -169,6 +195,8 @@ APR_DECLARE(apr_status_t) apr_thread_detach(apr_thread_t *thd)
169195
}
170196

171197
if (suspend_thread(thd->td) == B_NO_ERROR) {
198+
/* Detach from the parent pool too */
199+
apr__pool_unmanage(thd->pool);
172200
thd->detached = 1;
173201
return APR_SUCCESS;
174202
}

‎threadproc/netware/thread.c

+31-2
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,9 @@
2121

2222
static int thread_count = 0;
2323

24+
/* Internal (from apr_pools.c) */
25+
extern apr_status_t apr__pool_unmanage(apr_pool_t *pool);
26+
2427
apr_status_t apr_threadattr_create(apr_threadattr_t **new,
2528
apr_pool_t *pool)
2629
{
@@ -113,9 +116,32 @@ apr_status_t apr_thread_create(apr_thread_t **new,
113116
(*new)->data = data;
114117
(*new)->func = func;
115118
(*new)->thread_name = (char*)apr_pstrdup(pool, threadName);
119+
116120
(*new)->detached = (attr && apr_threadattr_detach_get(attr) == APR_DETACH);
117-
118-
stat = apr_pool_create(&(*new)->pool, pool);
121+
if ((*new)->detached) {
122+
stat = apr_pool_create_unmanaged_ex(&(*new)->pool,
123+
apr_pool_abort_get(pool),
124+
NULL);
125+
}
126+
else {
127+
/* The thread can be apr_thread_detach()ed later, so the pool needs
128+
* its own allocator to not depend on the parent pool which could be
129+
* destroyed before the thread exits. The allocator needs no mutex
130+
* obviously since the pool should not be used nor create children
131+
* pools outside the thread.
132+
*/
133+
apr_allocator_t *allocator;
134+
if (apr_allocator_create(&allocator) != APR_SUCCESS) {
135+
return APR_ENOMEM;
136+
}
137+
stat = apr_pool_create_ex(&(*new)->pool, pool, NULL, allocator);
138+
if (stat == APR_SUCCESS) {
139+
apr_allocator_owner_set(allocator, (*new)->pool);
140+
}
141+
else {
142+
apr_allocator_destroy(allocator);
143+
}
144+
}
119145
if (stat != APR_SUCCESS) {
120146
return stat;
121147
}
@@ -197,7 +223,10 @@ apr_status_t apr_thread_detach(apr_thread_t *thd)
197223
return APR_EINVAL;
198224
}
199225

226+
/* Detach from the parent pool too */
227+
apr__pool_unmanage(thd->pool);
200228
thd->detached = 1;
229+
201230
return APR_SUCCESS;
202231
}
203232

‎threadproc/os2/thread.c

+41-2
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,10 @@
2424
#include "apr_arch_file_io.h"
2525
#include <stdlib.h>
2626

27+
/* Internal (from apr_pools.c) */
28+
extern apr_status_t apr__pool_unmanage(apr_pool_t *pool);
29+
30+
2731
APR_DECLARE(apr_status_t) apr_threadattr_create(apr_threadattr_t **new, apr_pool_t *pool)
2832
{
2933
(*new) = (apr_threadattr_t *)apr_palloc(pool, sizeof(apr_threadattr_t));
@@ -95,8 +99,40 @@ APR_DECLARE(apr_status_t) apr_thread_create(apr_thread_t **new, apr_threadattr_t
9599
thread->attr = attr;
96100
thread->func = func;
97101
thread->data = data;
98-
stat = apr_pool_create(&thread->pool, pool);
99-
102+
103+
if (attr && attr->attr & APR_THREADATTR_DETACHED) {
104+
stat = apr_pool_create_unmanaged_ex(&thread->pool,
105+
apr_pool_abort_get(pool),
106+
NULL);
107+
}
108+
else {
109+
/* The thread can be apr_thread_detach()ed later, so the pool needs
110+
* its own allocator to not depend on the parent pool which could be
111+
* destroyed before the thread exits. The allocator needs no mutex
112+
* obviously since the pool should not be used nor create children
113+
* pools outside the thread.
114+
*/
115+
apr_allocator_t *allocator;
116+
if (apr_allocator_create(&allocator) != APR_SUCCESS) {
117+
return APR_ENOMEM;
118+
}
119+
stat = apr_pool_create_ex(&thread->pool, pool, NULL, allocator);
120+
if (stat == APR_SUCCESS) {
121+
apr_thread_mutex_t *mutex;
122+
stat = apr_thread_mutex_create(&mutex, APR_THREAD_MUTEX_DEFAULT,
123+
thread->pool);
124+
if (stat == APR_SUCCESS) {
125+
apr_allocator_mutex_set(allocator, mutex);
126+
apr_allocator_owner_set(allocator, thread->pool);
127+
}
128+
else {
129+
apr_pool_destroy(thread->pool);
130+
}
131+
}
132+
if (stat != APR_SUCCESS) {
133+
apr_allocator_destroy(allocator);
134+
}
135+
}
100136
if (stat != APR_SUCCESS) {
101137
return stat;
102138
}
@@ -172,7 +208,10 @@ APR_DECLARE(apr_status_t) apr_thread_detach(apr_thread_t *thd)
172208
return APR_EINVAL;
173209
}
174210

211+
/* Detach from the parent pool too */
212+
apr__pool_unmanage(thd->pool);
175213
thd->attr->attr |= APR_THREADATTR_DETACHED;
214+
176215
return APR_SUCCESS;
177216
}
178217

‎threadproc/unix/thread.c

+30-1
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,9 @@
2222

2323
#if APR_HAVE_PTHREAD_H
2424

25+
/* Internal (from apr_pools.c) */
26+
extern apr_status_t apr__pool_unmanage(apr_pool_t *pool);
27+
2528
/* Destroy the threadattr object */
2629
static apr_status_t threadattr_cleanup(void *data)
2730
{
@@ -172,8 +175,32 @@ APR_DECLARE(apr_status_t) apr_thread_create(apr_thread_t **new,
172175

173176
(*new)->data = data;
174177
(*new)->func = func;
178+
175179
(*new)->detached = (attr && apr_threadattr_detach_get(attr) == APR_DETACH);
176-
stat = apr_pool_create(&(*new)->pool, pool);
180+
if ((*new)->detached) {
181+
stat = apr_pool_create_unmanaged_ex(&(*new)->pool,
182+
apr_pool_abort_get(pool),
183+
NULL);
184+
}
185+
else {
186+
/* The thread can be apr_thread_detach()ed later, so the pool needs
187+
* its own allocator to not depend on the parent pool which could be
188+
* destroyed before the thread exits. The allocator needs no mutex
189+
* obviously since the pool should not be used nor create children
190+
* pools outside the thread.
191+
*/
192+
apr_allocator_t *allocator;
193+
if (apr_allocator_create(&allocator) != APR_SUCCESS) {
194+
return APR_ENOMEM;
195+
}
196+
stat = apr_pool_create_ex(&(*new)->pool, pool, NULL, allocator);
197+
if (stat == APR_SUCCESS) {
198+
apr_allocator_owner_set(allocator, (*new)->pool);
199+
}
200+
else {
201+
apr_allocator_destroy(allocator);
202+
}
203+
}
177204
if (stat != APR_SUCCESS) {
178205
return stat;
179206
}
@@ -253,6 +280,8 @@ APR_DECLARE(apr_status_t) apr_thread_detach(apr_thread_t *thd)
253280
#else
254281
if ((stat = pthread_detach(*thd->td)) == 0) {
255282
#endif
283+
/* Detach from the parent pool too */
284+
apr__pool_unmanage(thd->pool);
256285
thd->detached = 1;
257286

258287
return APR_SUCCESS;

‎threadproc/win32/thread.c

+30-1
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,9 @@
2828
/* Chosen for us by apr_initialize */
2929
DWORD tls_apr_thread = 0;
3030

31+
/* Internal (from apr_pools.c) */
32+
extern apr_status_t apr__pool_unmanage(apr_pool_t *pool);
33+
3134
APR_DECLARE(apr_status_t) apr_threadattr_create(apr_threadattr_t **new,
3235
apr_pool_t *pool)
3336
{
@@ -103,7 +106,31 @@ APR_DECLARE(apr_status_t) apr_thread_create(apr_thread_t **new,
103106

104107
(*new)->data = data;
105108
(*new)->func = func;
106-
stat = apr_pool_create(&(*new)->pool, pool);
109+
110+
if (attr && attr->detach) {
111+
stat = apr_pool_create_unmanaged_ex(&(*new)->pool,
112+
apr_pool_abort_get(pool),
113+
NULL);
114+
}
115+
else {
116+
/* The thread can be apr_thread_detach()ed later, so the pool needs
117+
* its own allocator to not depend on the parent pool which could be
118+
* destroyed before the thread exits. The allocator needs no mutex
119+
* obviously since the pool should not be used nor create children
120+
* pools outside the thread.
121+
*/
122+
apr_allocator_t *allocator;
123+
if (apr_allocator_create(&allocator) != APR_SUCCESS) {
124+
return APR_ENOMEM;
125+
}
126+
stat = apr_pool_create_ex(&(*new)->pool, pool, NULL, allocator);
127+
if (stat == APR_SUCCESS) {
128+
apr_allocator_owner_set(allocator, (*new)->pool);
129+
}
130+
else {
131+
apr_allocator_destroy(allocator);
132+
}
133+
}
107134
if (stat != APR_SUCCESS) {
108135
return stat;
109136
}
@@ -187,6 +214,8 @@ APR_DECLARE(apr_status_t) apr_thread_detach(apr_thread_t *thd)
187214
}
188215

189216
if (CloseHandle(thd->td)) {
217+
/* Detach from the parent pool too */
218+
apr__pool_unmanage(thd->pool);
190219
thd->td = NULL;
191220
return APR_SUCCESS;
192221
}

0 commit comments

Comments
 (0)
Please sign in to comment.