[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: Unbounded memory usage in mod_dav + mod_headers/mod_deflate/...

From: Joe Orton <jorton_at_redhat.com>
Date: Mon, 22 Aug 2016 16:19:18 +0100

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
Received on 2016-08-22 17:19:26 CEST

This is an archived mail posted to the Subversion Dev mailing list.