You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@zookeeper.apache.org by Paul Asmuth <pa...@googlemail.com> on 2016/06/06 11:00:25 UTC
Segfaults and unitialized mutexes in zookeeper c library
Hey,
I encountered some issues with the zookeepeer c client.
The problem starts in the zookeeper_init_internal method. A lot of
initialization work is performed here and if any of the initialization
routines fails, the code jumps to the "abort" label to perform various
cleanup tasks [1]. The conceptual issue is that a bunch of the cleanup code
tries to take locks on the zk structure that are only intialized in
adaptor_init in line 1181 (at the very end of the zookeeper_init_internal
method) [2]. So if we fail before reaching adaptor_init this causes trouble.
One specific instance of an invalid memory access that this causes is in
free_completions [3]. Here, in line 1651 zoo_lock_auth will fail because it
tries to grab an invalid mutex, after which the a_list struct is
uninitialized (the linked list next pointer points to random memory) and
subsequently the free routine segfaults.
An easy way to trigger this bug-path is to pass an invalid hostname, or do
anything else that causes the zookeeper_init_internal method to fail before
adaptor_init.
In my local checkout/codebase, I have added correct initialization for the
a_list struct in the free_completions routine, which at least fixes the
segfault for now. However this still leaves the issue that the cleanup code
tries to grab a lot of invalid locks, which all fail. I think in order to
fix this properly, one would need to do a larger refactoring of the code
(add another adaptor_preinit routine to the adaptor interface maybe?) and I
wasn't sure if that would be appreciated, so I didn't attach a patch for
now. If someone wants me to try and clean this up, I would be happy to give
it a try.
best regards
~ paul
[1]
https://github.com/apache/zookeeper/blob/trunk/src/c/src/zookeeper.c#L1078
[2]
https://github.com/apache/zookeeper/blob/trunk/src/c/src/zookeeper.c#L1181
[3]
https://github.com/apache/zookeeper/blob/trunk/src/c/src/zookeeper.c#L1651
------------------
BACKTRACE
Program received signal SIGSEGV, Segmentation fault.
0x000000010004f6d5 in free_auth_completion (a_list=0x7fff5fbff048) at
/deps/3rdparty/zookeeper/source/src/zookeeper.c:260
260 tmp = tmp->next;
#0 0x000000010004f6d5 in free_auth_completion (a_list=0x7fff5fbff048) at
/deps/3rdparty/zookeeper/source/src/zookeeper.c:260
#1 0x000000010004f500 in free_completions (zh=0x1003022f0,
callCompletion=1, reason=-116) at
/deps/3rdparty/zookeeper/source/src/zookeeper.c:1219
#2 0x0000000100057bfd in cleanup_bufs (zh=0x1003022f0, callCompletion=1,
rc=-116) at /deps/3rdparty/zookeeper/source/src/zookeeper.c:1227
#3 0x000000010004ee42 in destroy (zh=0x1003022f0) at
/deps/3rdparty/zookeeper/source/src/zookeeper.c:393
#4 0x000000010004eaf3 in zookeeper_init (host=0x1006005b0
"xxxinvalidhostname:2181", watcher=0x100007670 <xxx::zk_watch_cb(_zhandle*,
int, int, char const*, void*)>,
recv_timeout=10000, clientid=0x0, context=0x100600350, flags=0) at
/deps/3rdparty/zookeeper/source/src/zookeeper.c:877
--
Paul Asmuth
M: +44-(0)7827-808651
paul@asmuth.com
45 Comeragh Rd.
W149HT London
Re: Segfaults and unitialized mutexes in zookeeper c library
Posted by Paul Asmuth <pa...@googlemail.com>.
PS: I think this bug was introduced in SVN #1719528, which - as it seems -
tried to work around the uninitialized locks problem by adding an int
return code to all the lock_xxx functions, allowing them to indicate a
failure. The change introduce the invalid memory access since some (always
required) init code is only run after the lock was obtained successfully.
However, I think there is a much large issue with the change and I think it
must be reverted. Trying to lock an uninitialized mutex is undefined
behaviour on POSIX and may lead to deadlocks, etc.
>> If *mutex* does not refer to an initialized mutex object, the behavior of
*pthread_mutex_lock*(), *pthread_mutex_trylock*(), and
*pthread_mutex_unlock*() is undefined.
http://pubs.opengroup.org/onlinepubs/9699919799/functions/pthread_mutex_lock.html
On Mon, Jun 6, 2016 at 1:00 PM, Paul Asmuth <pa...@googlemail.com>
wrote:
> Hey,
>
> I encountered some issues with the zookeepeer c client.
>
> The problem starts in the zookeeper_init_internal method. A lot of
> initialization work is performed here and if any of the initialization
> routines fails, the code jumps to the "abort" label to perform various
> cleanup tasks [1]. The conceptual issue is that a bunch of the cleanup code
> tries to take locks on the zk structure that are only intialized in
> adaptor_init in line 1181 (at the very end of the zookeeper_init_internal
> method) [2]. So if we fail before reaching adaptor_init this causes trouble.
>
> One specific instance of an invalid memory access that this causes is in
> free_completions [3]. Here, in line 1651 zoo_lock_auth will fail because it
> tries to grab an invalid mutex, after which the a_list struct is
> uninitialized (the linked list next pointer points to random memory) and
> subsequently the free routine segfaults.
>
> An easy way to trigger this bug-path is to pass an invalid hostname, or do
> anything else that causes the zookeeper_init_internal method to fail before
> adaptor_init.
>
> In my local checkout/codebase, I have added correct initialization for the
> a_list struct in the free_completions routine, which at least fixes the
> segfault for now. However this still leaves the issue that the cleanup code
> tries to grab a lot of invalid locks, which all fail. I think in order to
> fix this properly, one would need to do a larger refactoring of the code
> (add another adaptor_preinit routine to the adaptor interface maybe?) and I
> wasn't sure if that would be appreciated, so I didn't attach a patch for
> now. If someone wants me to try and clean this up, I would be happy to give
> it a try.
>
> best regards
> ~ paul
>
> [1]
> https://github.com/apache/zookeeper/blob/trunk/src/c/src/zookeeper.c#L1078
> [2]
> https://github.com/apache/zookeeper/blob/trunk/src/c/src/zookeeper.c#L1181
> [3]
> https://github.com/apache/zookeeper/blob/trunk/src/c/src/zookeeper.c#L1651
>
> ------------------
>
> BACKTRACE
>
> Program received signal SIGSEGV, Segmentation fault.
>
> 0x000000010004f6d5 in free_auth_completion (a_list=0x7fff5fbff048) at
> /deps/3rdparty/zookeeper/source/src/zookeeper.c:260
> 260 tmp = tmp->next;
>
> #0 0x000000010004f6d5 in free_auth_completion (a_list=0x7fff5fbff048) at
> /deps/3rdparty/zookeeper/source/src/zookeeper.c:260
> #1 0x000000010004f500 in free_completions (zh=0x1003022f0,
> callCompletion=1, reason=-116) at
> /deps/3rdparty/zookeeper/source/src/zookeeper.c:1219
> #2 0x0000000100057bfd in cleanup_bufs (zh=0x1003022f0, callCompletion=1,
> rc=-116) at /deps/3rdparty/zookeeper/source/src/zookeeper.c:1227
> #3 0x000000010004ee42 in destroy (zh=0x1003022f0) at
> /deps/3rdparty/zookeeper/source/src/zookeeper.c:393
> #4 0x000000010004eaf3 in zookeeper_init (host=0x1006005b0
> "xxxinvalidhostname:2181", watcher=0x100007670 <xxx::zk_watch_cb(_zhandle*,
> int, int, char const*, void*)>,
> recv_timeout=10000, clientid=0x0, context=0x100600350, flags=0) at
> /deps/3rdparty/zookeeper/source/src/zookeeper.c:877
>
>
> --
> Paul Asmuth
> M: +44-(0)7827-808651
> paul@asmuth.com
>
> 45 Comeragh Rd.
> W149HT London
>
--
Paul Asmuth
M: +44-(0)7827-808651
paul@asmuth.com
45 Comeragh Rd.
W149HT London