Mailing List Archive: [Regression in httpd 2.4.52] mod_dav: Potentially unbounded memory usage in PROPFIND with dav_get_props() and dav_propfind

[Regression in httpd 2.4.52] mod_dav: Potentially unbounded memory usage in PROPFIND with dav_get_props() and dav_propfind_walker()

evgeny.kotkov at visualsvn

Jan 14, 2022, 4:57 AM

Post #1 of 4 (308 views)

Permalink

Hi,

I might have stumbled across a regression in httpd 2.4.52 where mod_dav was
changed in a way where dav_get_props() now allocates data in resource->pool.

I think that r1879889 [1] is the change that is causing the new behavior.
This change has been backported to 2.4.x in r1895893 [2].

Consider the new part of the dav_get_props() function:

DAV_DECLARE(dav_get_props_result) dav_get_props()
{
…
element = apr_pcalloc(propdb->resource->pool, sizeof(dav_liveprop_elem));
element->doc = doc;
…

This code unconditionally allocates data in resource->pool (this is the only
such place, other allocations performed by this function happen in propdb->p).

The dav_get_props() is called by dav_propfind_walker(). The walker function
is driven by a specific provider. Both of the two implementations known to
me, mod_dav_fs and mod_dav_svn, happen to use a long-living pool as the
resource->pool during the walk.

I think that with this change, the PROPFIND walks are going to make O(N)
allocations for O(N) walked items — which is an unbounded memory usage
and a regression, compared to 2.4.51.

[1] https://svn.apache.org/r1879889
[2] https://svn.apache.org/r1895893

Thanks,
Evgeny Kotkov

Re: [Regression in httpd 2.4.52] mod_dav: Potentially unbounded memory usage in PROPFIND with dav_get_props() and dav_propfind_walker() [ In reply to ]

rpluem at apache

Jan 14, 2022, 5:01 AM

Post #2 of 4 (308 views)

Permalink

On 1/14/22 1:57 PM, Evgeny Kotkov wrote:
> Hi,
>
> I might have stumbled across a regression in httpd 2.4.52 where mod_dav was
> changed in a way where dav_get_props() now allocates data in resource->pool.
>
> I think that r1879889 [1] is the change that is causing the new behavior.
> This change has been backported to 2.4.x in r1895893 [2].
>
> Consider the new part of the dav_get_props() function:
>
> DAV_DECLARE(dav_get_props_result) dav_get_props()
> {
> …
> element = apr_pcalloc(propdb->resource->pool, sizeof(dav_liveprop_elem));
> element->doc = doc;
> …
>
> This code unconditionally allocates data in resource->pool (this is the only
> such place, other allocations performed by this function happen in propdb->p).
>
> The dav_get_props() is called by dav_propfind_walker(). The walker function
> is driven by a specific provider. Both of the two implementations known to
> me, mod_dav_fs and mod_dav_svn, happen to use a long-living pool as the
> resource->pool during the walk.
>
> I think that with this change, the PROPFIND walks are going to make O(N)
> allocations for O(N) walked items — which is an unbounded memory usage
> and a regression, compared to 2.4.51.
>
> [1] https://svn.apache.org/r1879889
> [2] https://svn.apache.org/r1895893

Can you please check if the below patch fixes your issue?

Index: modules/dav/main/props.c
===================================================================
--- modules/dav/main/props.c (revision 1896810)
+++ modules/dav/main/props.c (working copy)
@@ -805,9 +805,15 @@
/* we lose both the document and the element when calling (insert_prop),
* make these available in the pool.
*/
- element = apr_pcalloc(propdb->resource->pool, sizeof(dav_liveprop_elem));
+ element = dav_get_liveprop_element(propdb->resource);
+ if (!element) {
+ element = apr_pcalloc(propdb->resource->pool, sizeof(dav_liveprop_elem));
+ apr_pool_userdata_setn(element, DAV_PROP_ELEMENT, NULL, propdb->resource->pool);
+ }
+ else {
+ memset(element, 0, sizeof(dav_liveprop_elem));
+ }
element->doc = doc;
- apr_pool_userdata_setn(element, DAV_PROP_ELEMENT, NULL, propdb->resource->pool);

/* ### NOTE: we should pass in TWO buffers -- one for keys, one for
the marks */

Regards

Rüdiger

Re: [Regression in httpd 2.4.52] mod_dav: Potentially unbounded memory usage in PROPFIND with dav_get_props() and dav_propfind_walker() [ In reply to ]

evgeny.kotkov at visualsvn

Jan 18, 2022, 5:58 AM

Post #3 of 4 (307 views)

Permalink

Ruediger Pluem <rpluem@apache.org> writes:

> Can you please check if the below patch fixes your issue?

I have to say that the reason and the original intention of using
resource->pool's userdata here are still somewhat unclear to me.

But it does look like the patch performs the allocation only once per
resource->pool, and so it should fix the unbounded memory usage issue.

Thanks!

Regards,
Evgeny Kotkov

Re: [Regression in httpd 2.4.52] mod_dav: Potentially unbounded memory usage in PROPFIND with dav_get_props() and dav_propfind_walker() [ In reply to ]

rpluem at apache

Jan 18, 2022, 12:13 PM

Post #4 of 4 (306 views)

Permalink

On 1/18/22 2:58 PM, Evgeny Kotkov wrote:
> Ruediger Pluem <rpluem@apache.org> writes:
>
>> Can you please check if the below patch fixes your issue?
>
> I have to say that the reason and the original intention of using
> resource->pool's userdata here are still somewhat unclear to me.

An application seems to be here:

https://github.com/minfrin/mod_dav_calendar/blob/main/mod_dav_calendar.c#L2170

As far as I understand the code needs the data stored in the userdata and has no other means to retrieve it.

>
> But it does look like the patch performs the allocation only once per
> resource->pool, and so it should fix the unbounded memory usage issue.

r1897182

Regards

Rüdiger