httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Brian Pane <brian.p...@cnet.com>
Subject Re: Core dump in mod-include
Date Fri, 14 Dec 2001 18:47:09 GMT
Justin Erenkrantz wrote:

>On Fri, Dec 14, 2001 at 09:32:11AM -0800, Ian Holsman wrote:
>
>>hi guys.
>>we've been investigating a couple of core dumps in some testing and we 
>>think we found one in mod-include.
>>
>>here is the email thread.
>>does anyone have any reason NOT to reset the state when reseting
>>the bytes parsed ?
>>
>
>Just to make sure I understand what you guys are thinking of,
>this is your proposed patch, right?  (more follows)
>
>Index: modules/filters/mod_include.c
>===================================================================
>RCS file: /home/cvs/httpd-2.0/modules/filters/mod_include.c,v
>retrieving revision 1.166
>diff -u -r1.166 mod_include.c
>--- modules/filters/mod_include.c	2001/12/08 03:14:50	1.166
>+++ modules/filters/mod_include.c	2001/12/14 18:12:31
>@@ -3066,6 +3066,7 @@
>     }
>     else {
>         ctx->bytes_parsed = 0;
>+        ctx->state = PRE_HEAD;
>     }
> 
>     if ((parent = ap_get_module_config(r->request_config, &include_module))) {
>
>If so, then I'm kind of leery about this.
>
>The reason is that filters can be called many times over the
>lifetime of the data.  So, we could leave the parse engine in a 
>ctx->state = POST_HEAD between calls because we are still waiting
>for a bucket-brigade containing the rest of the tag.  We'd obliterate
>that value with this.  =)  So, I don't think resetting the state is 
>going to do you any good here.
>
>Based on your description, what seems to be happening is that the
>client either disconnects or timeouts and the finalize_request is
>passing down an EOS but mod_include is stuck in some internal
>state.  mod_include just isn't handling it well.
>
>So, I'm thinking the appropriate check is to stop at EOS or 
>SENTINEL.  There is no good reason to continue processing if we 
>see an EOS.  But, we have a potentially saved brigade to handle.
>So, what do we do?  Perhaps this patch might be better:
>

I haven't had time to read through through Justin's patch in detail,
but conceptually I think it's the right approach: check for the EOS
condition as a special case in the end-of-brigade code that's crashing,
rather than resetting the state every time we enter the filter.

--Brian




Mime
View raw message