mod_cgid: fix a hash table corruption problem which could

result in the wrong script being cleaned up at the end of a
request.

Unique storage was not used for the key, as the code assumed
incorrectly that apr_hash_set() made a copy of the key.  Thus,
when the script pid was looked up at the end of the request,
some other script's pid could be found.

git-svn-id: https://svn.apache.org/repos/asf/httpd/httpd/trunk@101374 13f79535-47bb-0310-9956-ffa450edef68

diff --git a/CHANGES b/CHANGES
index 94bb6da4e4d79e43b6e5e1755cfbb2fd871087fd..6b2b5eda6ba9bdee0840ac965ca7faf415946cb0 100644 (file)
--- a/CHANGES
+++ b/CHANGES
@@ -2,6 +2,10 @@ Changes with Apache 2.1.0-dev
 
   [Remove entries to the current 2.0 section below, when backported]
 
+  *) mod_cgid: fix a hash table corruption problem which could
+     result in the wrong script being cleaned up at the end of a
+     request.  [Jeff Trawick]
+
   *) Log an error when requests for URIs which fail to map to a valid 
      filesystem name are rejected with 403.  [Jeff Trawick]
 

diff --git a/modules/generators/mod_cgid.c b/modules/generators/mod_cgid.c
index 11bdc427fb9bf4b8eb17016a3b4810dce585cf19..a174e5d7b7e0bd6b6b3de6e9fb283d8ec0b355c3 100644 (file)
--- a/modules/generators/mod_cgid.c
+++ b/modules/generators/mod_cgid.c
@@ -778,7 +778,26 @@ static int cgid_server(void *data)
                              apr_filepath_name_get(r->filename));
             }
             else {
-                apr_hash_set(script_hash, &cgid_req.conn_id, sizeof(cgid_req.conn_id), 
+                /* We don't want to leak storage for the key, so only allocate
+                 * a key if the key doesn't exist yet in the hash; there are
+                 * only a limited number of possible keys (one for each
+                 * possible thread in the server), so we can allocate a copy
+                 * of the key the first time a thread has a cgid request.
+                 * Note that apr_hash_set() only uses the storage passed in
+                 * for the key if it is adding the key to the hash for the
+                 * first time; new key storage isn't needed for replacing the
+                 * existing value of a key.
+                 */
+                void *key;
+
+                if (apr_hash_get(script_hash, &cgid_req.conn_id, sizeof(cgid_req.conn_id))) {
+                    key = &cgid_req.conn_id;
+                }
+                else {
+                    key = apr_pcalloc(pcgi, sizeof(cgid_req.conn_id));
+                    memcpy(key, &cgid_req.conn_id, sizeof(cgid_req.conn_id));
+                }
+                apr_hash_set(script_hash, key, sizeof(cgid_req.conn_id),
                              (void *)procnew->pid);
             }
         }