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

Re: [PATCH] fix use-after-free in mod_dav_svn's log_warning()

From: Stefan Fuhrmann <stefan2_at_apache.org>
Date: Tue, 18 Dec 2018 03:55:42 +0100

On 17.12.18 10:44, Stefan Sperling wrote:
> I have hit a use-after-free in mod_dav_svn while running SVN's regression
> tests on OpenBSD with httpd 2.4. This problem was apparently known to the
> author; see the comment which is removed in the diff below.
>
> In short, the request structure used as logging context can already be
> freed before log_warning() runs. The repository structure is allocated
> in the request's pool, which means the repository is closed when the
> request pool gets freed. I am not sure which, if any, ordering guarantess
> exist in this situation. But the patch below switches the logging context
> to the connection instead of the request and I have been unable to reproduce
> the issue since.
>
> Is there a better solution?
> Does my proposed solution lose too much logging context?
Error handling during tear-down is always a bit messy.

I guess not using the request struct will mean we cannot
tell what operation cased a problem - ever (not sure,
though). If true, that's a high cost.

One way to improve your solution would be to make
the error handling degrade as the objects are being
destroyed. But it adds / duplicates a bit of code:

* keep the logging based on request context
* apr_pool_pre_cleanup_register on the request pool,
   a switch to the new connection-based logging

Greetings from CN!
-- Stefan^2
Received on 2018-12-18 03:55:55 CET

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