Mailing-List: contact dev-help@httpd.apache.org; run by ezmlm
Precedence: bulk
Reply-To: dev@httpd.apache.org
Sender: trawick@rdu163-40-092.nc.rr.com
To: dev@httpd.apache.org
Subject: Re: [PATCH] fix segfault in core input filter
References: <m31ygxm1c7.fsf@rdu163-40-092.nc.rr.com>
	<20020111155236.GA14870@ebuilt.com>
	<m38zb4iwuz.fsf@rdu163-40-092.nc.rr.com>
From: Jeff Trawick <trawick@attglobal.net>
Date: 11 Jan 2002 13:26:46 -0500
In-Reply-To: <m38zb4iwuz.fsf@rdu163-40-092.nc.rr.com>
Message-ID: <m3ofk0d9qx.fsf@rdu163-40-092.nc.rr.com>
Lines: 78
User-Agent: Gnus/5.0808 (Gnus v5.8.8) Emacs/20.7
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii

Jeff Trawick <trawick@attglobal.net> writes:

> I'm following what happens when we get EOF on socket...
> 
> socket_read() returns APR_SUCCESS and immortal bucket with ""
> 
> core-input doesn't error out since it got APR_SUCCESS
> 
> core-input walks over the "" and deletes the immortal bucket
> 
> core-input sees empty brigade and returns APR_EOF
> 
> it gets returned up through net_time_filter to check_pipeline_flush
> 
> check_pipeline_flush knows ap_get_brigade() failed but it lost the
> retcode and has no way to return the error anyway
> 
> return to caller (ap_process_request)
> 
> do ap_run_log_transaction
> 
> back to ap_process_http_connection
> 
> ap_process_http_connection calls ap_read_request again and we segfault
> since core-input's brigade was empty
> 
> --/--
> 
> if check_pipeline_flush cleared c->keepalive we wouldn't have tried to
> read another request

The patch below gets rid of a segfault while processing every HTTP/1.1 
request (w/ ElectricFence and APR_POOL_DEBUG) but there is a much less
infrequent segfault remaining :(

Index: modules/http/http_request.c
===================================================================
RCS file: /home/cvs/httpd-2.0/modules/http/http_request.c,v
retrieving revision 1.122
diff -u -r1.122 http_request.c
--- modules/http/http_request.c	14 Dec 2001 03:29:13 -0000	1.122
+++ modules/http/http_request.c	11 Jan 2002 18:26:04 -0000
@@ -240,6 +240,7 @@
        ### need to send a FLUSH. */
     apr_bucket_brigade *bb = apr_brigade_create(r->pool);
     apr_off_t zero = 0;
+    apr_status_t rv = APR_SUCCESS;
 
     /* Flush the filter contents if:
      *
@@ -249,7 +250,7 @@
     /* ### shouldn't this read from the connection input filters? */
     /* ### is zero correct? that means "read one line" */
     if (!r->connection->keepalive || 
-        ap_get_brigade(r->input_filters, bb, AP_MODE_PEEK, &zero) != APR_SUCCESS) {
+        (rv = ap_get_brigade(r->input_filters, bb, AP_MODE_PEEK, &zero)) != APR_SUCCESS) {
         apr_bucket *e = apr_bucket_flush_create();
 
         /* We just send directly to the connection based filters.  At
@@ -260,6 +261,12 @@
          */
         APR_BRIGADE_INSERT_HEAD(bb, e);
         ap_pass_brigade(r->connection->output_filters, bb);
+        if (rv != APR_SUCCESS) {
+            /* We got an error earlier reading from the socket.  Make sure
+             * keepalive is off so that we don't try to read another request.
+             */
+            r->connection->keepalive = 0;
+        }
     }
 }
 

-- 
Jeff Trawick | trawick@attglobal.net | PGP public key at web site:
       http://www.geocities.com/SiliconValley/Park/9289/
             Born in Roswell... married an alien...