You are viewing a plain text version of this content. The canonical link for it is here.
Posted to cvs@httpd.apache.org by tr...@apache.org on 2005/10/08 02:10:05 UTC
svn commit: r307221 - in /httpd/httpd/trunk: CHANGES
server/mpm/worker/worker.c
Author: trawick
Date: Fri Oct 7 17:10:02 2005
New Revision: 307221
URL: http://svn.apache.org/viewcvs?rev=307221&view=rev
Log:
use Greg's cleaner fix for CAN-2005-2970
Modified:
httpd/httpd/trunk/CHANGES
httpd/httpd/trunk/server/mpm/worker/worker.c
Modified: httpd/httpd/trunk/CHANGES
URL: http://svn.apache.org/viewcvs/httpd/httpd/trunk/CHANGES?rev=307221&r1=307220&r2=307221&view=diff
==============================================================================
--- httpd/httpd/trunk/CHANGES [utf-8] (original)
+++ httpd/httpd/trunk/CHANGES [utf-8] Fri Oct 7 17:10:02 2005
@@ -47,7 +47,7 @@
*) SECURITY: CAN-2005-2970 (cve.mitre.org)
worker MPM: Fix a memory leak which can occur after an aborted
- connection in some limited circumstances. [Greg Ames, Jeff Trawick]
+ connection in some limited circumstances. [Greg Ames]
*) Doxygen fixup [Neale Ranns <neale ranns.org>, Ian Holsman]
Modified: httpd/httpd/trunk/server/mpm/worker/worker.c
URL: http://svn.apache.org/viewcvs/httpd/httpd/trunk/server/mpm/worker/worker.c?rev=307221&r1=307220&r2=307221&view=diff
==============================================================================
--- httpd/httpd/trunk/server/mpm/worker/worker.c (original)
+++ httpd/httpd/trunk/server/mpm/worker/worker.c Fri Oct 7 17:10:02 2005
@@ -583,8 +583,7 @@
int process_slot = ti->pid;
apr_pool_t *tpool = apr_thread_pool_get(thd);
void *csd = NULL;
- apr_pool_t *ptrans; /* Pool for per-transaction stuff */
- apr_pool_t *recycled_pool = NULL;
+ apr_pool_t *ptrans = NULL; /* Pool for per-transaction stuff */
apr_pollset_t *pollset;
apr_status_t rv;
ap_listen_rec *lr;
@@ -624,8 +623,11 @@
if (listener_may_exit) break;
if (!have_idle_worker) {
+ /* the following pops a recycled ptrans pool off a stack
+ * if there is one, in addition to reserving a worker thread
+ */
rv = ap_queue_info_wait_for_idler(worker_queue_info,
- &recycled_pool);
+ &ptrans);
if (APR_STATUS_IS_EOF(rv)) {
break; /* we've been signaled to die now */
}
@@ -713,8 +715,9 @@
} /* if/else */
if (!listener_may_exit) {
- /* create a new transaction pool for each accepted socket */
- if (recycled_pool == NULL) {
+ if (ptrans == NULL) {
+ /* we can't use a recycled transaction pool this time.
+ * create a new transaction pool */
apr_allocator_t *allocator;
apr_allocator_create(&allocator);
@@ -722,10 +725,6 @@
apr_pool_create_ex(&ptrans, pconf, NULL, allocator);
apr_allocator_owner_set(allocator, ptrans);
}
- else {
- ptrans = recycled_pool;
- recycled_pool = NULL;
- }
apr_pool_tag(ptrans, "transaction");
rv = lr->accept_func(&csd, lr, ptrans);
/* later we trash rv and rely on csd to indicate success/failure */
@@ -761,15 +760,11 @@
apr_socket_close(csd);
ap_log_error(APLOG_MARK, APLOG_CRIT, rv, ap_server_conf,
"ap_queue_push failed");
- recycled_pool = ptrans;
}
else {
have_idle_worker = 0;
}
}
- else {
- recycled_pool = ptrans;
- }
}
else {
if ((rv = SAFE_ACCEPT(apr_proc_mutex_unlock(accept_mutex)))
@@ -823,6 +818,7 @@
free(ti);
ap_scoreboard_image->servers[process_slot][thread_slot].pid = ap_my_pid;
+ ap_scoreboard_image->servers[process_slot][thread_slot].tid = apr_os_thread_current();
ap_scoreboard_image->servers[process_slot][thread_slot].generation = ap_my_generation;
ap_update_child_status_from_indexes(process_slot, thread_slot, SERVER_STARTING, NULL);
Re: svn commit: r307221 - in /httpd/httpd/trunk: CHANGES server/mpm/worker/worker.c
Posted by Greg Ames <gr...@apache.org>.
trawick@apache.org wrote:
> use Greg's cleaner fix for CAN-2005-2970
>
> Modified:
> @@ -823,6 +818,7 @@
> free(ti);
>
> ap_scoreboard_image->servers[process_slot][thread_slot].pid = ap_my_pid;
> + ap_scoreboard_image->servers[process_slot][thread_slot].tid = apr_os_thread_current();
> ap_scoreboard_image->servers[process_slot][thread_slot].generation = ap_my_generation;
> ap_update_child_status_from_indexes(process_slot, thread_slot, SERVER_STARTING, NULL);
? a reasonable looking line of code, but should it be here?
Greg