Orivej Desh wrote on Tue, 23 May 2017 08:20 +0000:
> Hello,
>
> I noticed in dmesg that in my repository svnserve occasionally crashes.
> This happens at exit, so it is not visible to end users. I captured a
> few sessions at the svn protocol level that resulted in a crash; client
> commands are quite different in each one; sending an input that crashed
> `svnserve -t` again always crashes it; I did not manage to reproduce
> this in a new repository. I tested with subversion 1.9.0, 1.9.4 and
> subversion trunk, apr 1.3.0 and apr 1.5.2 with the same result.
>
I cannot reproduce this with apr 1.5.1-3 (debian), svn 1.9.4 (self compiled),
and clang 3.5, under 'make svnserveautocheck'. I use 1.9.4 because that's the
traceback you posted; let's do further analysis on trunk if the problem
reproduces there.
> Here is the AddressSanitizer report of the problem (with subversion
> 1.9.4 and apr 1.3.0). For me it seems that a piece of memory belonging
> to the object_pool is registered for run_cleanups in the main pool, then
> right before return from main() all child pools of the main pool are
> destroyed, then run_cleanups of the main pool calls object_ref_cleanup
> with a pointer to a freed memory. Is this correct? How is this
> supposed to work? Could you help me spot the difference between
> expected and unexpected paths of code concerning the issue?
>
The report says that the use-after-free occured inside the cleanup handler. It
doesn't say where the accessed object was allocated or freed; to get that info,
you'd have had to compile APR with pool debugging (--enable-pool-debug), then
the second and third traceback would have pointed to the site of the malloc()
and of the apr_pool_clear().
Cleanup handlers work as follows: «apr_pool_cleanup_register(pool, baton, f, g)»
calls «f(baton)» when POOL is cleared. If BATON had been free()d before POOL
is cleared, then F will access freed memory.
Therefore, there are a few options to fixing such issues. One could install the
cleanup handler F on a different pool, or duplicate the BATON object into a
different pool, handler is invoked), or sometimes to use apr_pool_pre_cleanup_register()
instead. I'm not sure which of these options is best in this case (not
familiar with this module).
The pool cleanup is registered in add_object_ref(). Can you break in that
function and confirm which of its two callsites is involved?
Thanks for the report!
Daniel
Received on 2017-05-23 14:41:50 CEST