httpd-48021

Version:

mod_fcgid-2.3.4

Failure link:

https://issues.apache.org/bugzilla/show_bug.cgi?id=48021

How it is diagnosed:

Source analysis (we cannot reproduce the failure).

Symptom(Failure):

Wrong result. Using apache 2.2.14 with mod_fcgid 2.3.4 and PHP code, apache corrupted user upload (POST) data. This only occurs when the data size is large.

Root Cause:

Concurrency bug.

Apache uses a temporary file to store the intermediate POST data. If the data is too large, Apache needs to used more than one ‘read’ to get the data and ‘write’ into intermediate file. However, between two consecutive ‘write’ to intermediate file, Apache unexpectedly ‘truncated’ it to size zero!

--- httpd/mod_fcgid/trunk/modules/fcgid/fcgid_bridge.c        2009/10/19 22:03:07        826828
+++ httpd/mod_fcgid/trunk/modules/fcgid/fcgid_bridge.c        2009/10/19 22:03:30        826829
static int add_request_body(... …) {

   for (... …) {

      … …

       /* A large file... */
        if (request_size > sconf->max_mem_request_len) {

               apr_size_t wrote_len;

               static const char *fd_key = "fcgid_fd";

               /* We do not have an intermediate file yet... */

               if (fd == NULL){

                   void *tmp;

                   /* Get a file from the pool (previously used) */

                   apr_pool_userdata_get(&tmp, fd_key, r->connection->pool);

                   fd = tmp;

              /* So the correct behavior should be, we only need to truncate

                  the file if we are reusing it from the pool! */

+  
+                  if (fd != NULL) {
+                       if ((rv = apr_file_trunc(fd, 0)) != APR_SUCCESS) {
+                            ap_log_rerror(APLOG_MARK, APLOG_WARNING, rv, r,
+                                          "mod_fcgid: can't truncate existing "
+                                          "temporary file");
+                            return HTTP_INTERNAL_SERVER_ERROR;
+                        }
+                    } // if (fd != NULL)

               }// if (fd == NULL)

        /* This is the function return value of apr_pool_userdata_get -- Apache

                   cannot find a file from the pool!. */

               if (fd == NULL) {

                   … …

       /* During incorrect exe, apache cannot find the tmp file from the pool,

          so it made a new tmp file. Then it wouldn’t call truncate below!

         So after it is created, the content was directly written to the file!

         However, since the file is large and the for loop needs more than

         once, during the 2nd time, it called truncate even when the tmp

         file already had some contents! */

                   rv = apr_file_mktemp(&fd, template, 0,

                                        r->connection->pool);

                   … ...

-                } else if (need_truncate) {

                                    <---- Log HERE (untested branch cond)!!!

-                    need_truncate = 0;

-                    apr_file_trunc(fd, 0);

-                    cur_pos = 0;

                 }


                /* Write request to tmp file */
                if ((rv =
                     apr_file_write_full(fd, (const void *) data, len,

Is there any log message?:

No.

How Errlog we automatically handle:

Log the untested branch decision. The basic block of truncation should not be entered during correct execution.

Discussion:

This is yet another bug caused by premature optimization. Apache should have truncated the file immediately once it decided to recycle back to the pool (for later reuse of the filedescriptor). Instead, it use lazy truncation that only truncate when the fd was reused. It should log at the truncation point!