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>
From: Jeff Trawick <trawick@attglobal.net>
Date: 11 Jan 2002 15:14:58 -0500
In-Reply-To: <20020111155236.GA14870@ebuilt.com>
Message-ID: <m37kqo4pbx.fsf@rdu163-40-092.nc.rr.com>
Lines: 57
User-Agent: Gnus/5.0808 (Gnus v5.8.8) Emacs/20.7
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii

Justin Erenkrantz <jerenkrantz@ebuilt.com> writes:

> On Fri, Jan 11, 2002 at 09:03:36AM -0500, Jeff Trawick wrote:
> > With current CVS code, prefork, APR_POOL_DEBUG, Linux, and
> > ElectricFence I readily hit a segfault on the APR_BRIGADE_NORMALIZE()
> > call in core input filter.  It segfaulted because the brigade was
> > empty.  With the following patch I can't seem to hit a segfault
> > anymore.
> > 
> > Perhaps the brigade shouldn't be empty for some reason...
> 
> If the brigade in core_input_filter is empty, it means that the
> socket (and its bucket) has disappeared on us.  Most likely,
> someone isn't paying attention to APR_EOF return values.  
> 
> We should return an error or at the very least APR_EOF if we
> see an empty brigade there.  But, it would be more useful to
> find out which input filter isn't paying attention.  -- justin

Does anybody have any concerns before I commit?

(I should run with ElectricFence more often...)

Index: server/core.c
===================================================================
RCS file: /home/cvs/httpd-2.0/server/core.c,v
retrieving revision 1.129
diff -u -r1.129 core.c
--- server/core.c	11 Jan 2002 18:55:27 -0000	1.129
+++ server/core.c	11 Jan 2002 20:13:48 -0000
@@ -3035,6 +3035,10 @@
         APR_BRIGADE_INSERT_TAIL(ctx->b, e);
         net->in_ctx = ctx;
     }
+    else if (APR_BRIGADE_EMPTY(ctx->b)) {
+        /* hit EOF on socket already */
+        return APR_EOF;
+    }
 
     /* ### This is bad. */
     APR_BRIGADE_NORMALIZE(ctx->b);
@@ -3114,9 +3118,6 @@
 
         AP_DEBUG_ASSERT(*readbytes > 0);
         
-        if (APR_BRIGADE_EMPTY(ctx->b))
-            return APR_EOF;
-
         e = APR_BRIGADE_FIRST(ctx->b);
         rv = apr_bucket_read(e, &str, &len, mode);
 

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