httpd-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Joe Orton <jor...@redhat.com>
Subject Re: Unbounded memory usage in mod_dav + mod_headers/mod_deflate/...
Date Mon, 22 Aug 2016 15:19:18 GMT
On Fri, Aug 19, 2016 at 04:44:21PM +0300, Evgeny Kotkov wrote:
...
> The problem is caused by how mod_dav passes the output filter list to its
> providers.  Please see the deliver() hook definition in mod_dav.h:1948 and
> its usage in mod_dav.c:888:
> 
>     /* Repository provider hooks */
>     struct dav_hooks_repository
>     {
>         ...
>         dav_error * (*deliver)(const dav_resource *resource,
>                                ap_filter_t *output);
> 
> The hook receives the current head of the output filter list for a particular
> request.  Certain filters, such as the one in mod_headers, are designed
> to perform the work only once.  When the work is done, a filter removes
> itself from the list.  If a filter is the first in the list, this updates the
> head of the linked list in request_rec (r->output_filters).

Ouch.  Yes, this is a nasty API issue.

> One way of solving the problem that I can think of is:

Can you work around the bug in mod_dav_svn by writing to 
output->r->output_filters each time rather than the passed-in "output" 
directly?  Or alternatively use a request_rec stashed in resource->info 
and simply reference r->output_filters from there?

Obviously that doesn't fix the underlying API issue, but it'd be worth 
validating as a (relatively simple?) alternative.

BTW, looking at mod_dav_svn:repos.c's deliver(), the lack of brigade 
reuse is potentially going to consume a lot of RAM too; abbreviated code 
like:

      block = apr_palloc(resource->pool, SVN__STREAM_CHUNK_SIZE);
      while (1) {
        serr = svn_stream_read_full(stream, block, &bufsize);
        bb = apr_brigade_create(resource->pool, output->c->bucket_alloc);
        if ((status = ap_pass_brigade(output, bb)) != APR_SUCCESS) {

... that brigade should be created before entering the loop; call 
apr_brigade_cleanup() after calling ap_pass_brigade() at the end of each 
iteration to ensure it's empty.

Regards, Joe

Mime
View raw message