Hi, On Tue, Oct 22, 2013 at 02:13:36PM +0200, Gert Doering wrote: > On Tue, Oct 22, 2013 at 01:40:18PM +0200, Gert Doering wrote:
(I seem to be talking to myself a lot, lately...)
> > I could use some ideas on how to debug this further - if valgrind isn't
> > complaining, normally our memory management should be OK, but why is
> > VSZ/RSZ still growing, then...?
>
> Arne brought up a good point. If we "lose" the memory inside the gc_*
> functions (as in: use the same gc block for a very long time, like
> "while OpenVPN runs", but *do* free it at the end), valgrind might not
> be able to see it - after all, we nicely cleaned up later - but we'll
> still grow out of bounds at runtime.
... and *that* seems to be the issue here. I've instrumented the gc_arena
to have a creation time (time_t) and a use_count, and the only one that
shows up at renegotiation time is the gc inside the "environment set",
with this call chain... (included for those that are curious :-) ).
#3 0x080714e8 in add_env_item (str=0x811287c "untrusted_ip6=::ffff:127.0.0.1",
do_alloc=true, list=0x80fc500,
gc=0x810a65c) at ../../../openvpn-testing/src/openvpn/misc.c:553
#4 0x0807156a in env_set_add_nolock (es=0x80fc4fc, str=0x811287c
"untrusted_ip6=::ffff:127.0.0.1")
at ../../../openvpn-testing/src/openvpn/misc.c:568
#5 0x080716b2 in env_set_add (es=0x80fc4fc, str=0x811287c
"untrusted_ip6=::ffff:127.0.0.1")
at ../../../openvpn-testing/src/openvpn/misc.c:613
#6 0x08071b96 in setenv_str_ex (es=0x80fc4fc, name=0xbfffd430 "untrusted_ip6",
value=0xbfffd3b0 "::ffff:127.0.0.1", name_include=32772, name_exclude=0,
name_replace=0 '\000',
value_include=128, value_exclude=0, value_replace=0 '\000') at
../../../openvpn-testing/src/openvpn/misc.c:784
#7 0x080719d6 in setenv_str (es=0x80fc4fc, name=0xbfffd430 "untrusted_ip6",
value=0xbfffd3b0 "::ffff:127.0.0.1")
at ../../../openvpn-testing/src/openvpn/misc.c:735
#8 0x080b1a7a in setenv_sockaddr (es=0x80fc4fc, name_prefix=0x80e2139
"untrusted", addr=0x810b20c, flags=1)
at ../../../openvpn-testing/src/openvpn/socket.c:2396
#9 0x080b1b7e in setenv_link_socket_actual (es=0x80fc4fc,
name_prefix=0x80e2139 "untrusted", act=0x810b20c,
flags=1) at ../../../openvpn-testing/src/openvpn/socket.c:2426
#10 0x080bde24 in setenv_untrusted (session=0x810b19c) at
../../../openvpn-testing/src/openvpn/ssl_verify.c:74
#11 0x080bf07a in verify_cert (session=0x810b19c, cert=0x812fa50, cert_depth=0)
at ../../../openvpn-testing/src/openvpn/ssl_verify.c:667
#12 0x080c050e in verify_callback (preverify_ok=1, ctx=0xbfffd748)
at ../../../openvpn-testing/src/openvpn/ssl_verify_openssl.c:84
Preliminary analysis: the problematic code seems to be
env_set_add_nolock (struct env_set *es, const char *str)
{
remove_env_item (str, es->gc == NULL, &es->list);
add_env_item ((char *)str, true, &es->list, es->gc);
}
... because add_env_item() will (have to) do...
add_env_item (char *str, const bool do_alloc, struct env_item **list, struct
gc_arena *gc)
{
...
ALLOC_OBJ_GC (item, struct env_item, gc);
item->string = do_alloc ? string_alloc (str, gc): str;
item->next = *list;
*list = item;
...
}
so while the environment list itself is not growing out of bounds, the
gc_arena used for storing the local copy of the string is (because these
are only ever cleaned up if gc_free() is destroying the whole arena,
which for *this* one is only happening at program end).
OTOH, the whole thing looks like it would work perfectly if es->gc
were just NULL (string_alloc() would then call malloc() instead, and
remove_env_item() would call free()). So maybe this can be solved fairly
easily without rewriting all of this...
(Doing this in a train which will arrive soon -> digging deeper on my
way back)
gert
--
USENET is *not* the non-clickable part of WWW!
//www.muc.de/~gert/
Gert Doering - Munich, Germany g...@greenie.muc.de
fax: +49-89-35655025 g...@net.informatik.tu-muenchen.de
pgpIN7AfTOVa8.pgp
Description: PGP signature
