Bad talloc magic value inside tls.c:sess_free_vps
(Starting a new thread to keep mailreader depth sane) #4 0x00007f666e6ef97a in _fr_talloc_fault (reason=0x36ad408378 "Bad talloc magic value - unknown value") at src/lib/debug.c:561 #5 0x00000036ad402df1 in talloc_abort_unknown_value (ptr=<value optimized out>) at ../talloc.c:341 #6 talloc_chunk_from_ptr (ptr=<value optimized out>) at ../talloc.c:360 #7 talloc_get_name (ptr=<value optimized out>) at ../talloc.c:1153 #8 0x00000036ad4057eb in _talloc_get_type_abort (ptr=0x7f662c091dd0, name=0x7f666e714bdb "VALUE_PAIR", location=0x7f666e714bc7 "src/lib/debug.c:817") at ../talloc.c:1206 #9 0x00007f666e6f0104 in fr_verify_vp (file=0x7f666e7144f0 "src/lib/cursor.c", line=151, vp=0x7f662c091dd0) at src/lib/debug.c:817 #10 0x00007f666e6eec3f in fr_cursor_next (cursor=0x7f666a584490) at src/lib/cursor.c:151 #11 0x00007f666e70b75e in pairfree (vps=0x7f666a584508) at src/lib/valuepair.c:169 #12 0x00000000004493d6 in sess_free_vps (parent=0x7f664c0b76f0, data_ptr=0x7f662c07b140, ad=0x7f664c0b78e8, idx=0, argl=0, argp=0x0) at src/main/tls.c:1986 #13 0x00000037a0c6a68a in ?? () from /usr/lib64/libcrypto.so.10 #14 0x00000037a7c466bc in SSL_SESSION_free () from /usr/lib64/libssl.so.10 #15 0x00000037a7c445cd in SSL_free () from /usr/lib64/libssl.so.10 #16 0x000000000044624f in session_close (ssn=0x7f6644063950) at src/main/tls.c:599 At a guess, this is wrong: https://github.com/FreeRADIUS/freeradius-server/blob/v3.0.x/src/modules/rlm_... ...because FR_TLS_EX_INDEX_TALLOC is used to parent the VPs paircopy'ed in: https://github.com/FreeRADIUS/freeradius-server/blob/v3.0.x/src/main/tls.c#L... I spent a bit of time looking at this earlier; the lifecycle of some of these objects is really really really hard to follow, both because OpenSSL is a bit weird, and because the naming of the FR TLS stuff is used in different ways to similarly-named objects inside OpenSSL. Summary for my own notes: OpenSSL: SSL_CTX* is an OpenSSL object containing the certs/keys/settings, that creates an... SSL* is an OpenSSL "connection" i.e. an SSL handshake, that creates or uses a cached... SSL_SESSION* is a negotiated OpenSSL master key & algos FreeRADIUS tls_session_t is an OpenSSL SSL* i.e. connection plus some bits i.e. the BIOs to feed into/out of OpenSS eap_handler_t is an EAP exchange; it's keyed off State which mutates on every round trip. It contains a bunch of eap counter stuff and the tls_session_t Given that sess_free_vps() is called by OpenSSL when the session goes away, I guess we don't need to parent the cached VPs to anything; ditto the cached certs? Certainly don't want to parent the vps/certs to handler or tls_session_t; those have shorter lifetimes than SSL_SESSION* objects.
On 24 Jun 2014, at 17:47, Phil Mayers <p.mayers@IMPERIAL.AC.UK> wrote:
(Starting a new thread to keep mailreader depth sane)
#4 0x00007f666e6ef97a in _fr_talloc_fault (reason=0x36ad408378 "Bad talloc magic value - unknown value") at src/lib/debug.c:561 #5 0x00000036ad402df1 in talloc_abort_unknown_value (ptr=<value optimized out>) at ../talloc.c:341 #6 talloc_chunk_from_ptr (ptr=<value optimized out>) at ../talloc.c:360 #7 talloc_get_name (ptr=<value optimized out>) at ../talloc.c:1153 #8 0x00000036ad4057eb in _talloc_get_type_abort (ptr=0x7f662c091dd0, name=0x7f666e714bdb "VALUE_PAIR", location=0x7f666e714bc7 "src/lib/debug.c:817") at ../talloc.c:1206 #9 0x00007f666e6f0104 in fr_verify_vp (file=0x7f666e7144f0 "src/lib/cursor.c", line=151, vp=0x7f662c091dd0) at src/lib/debug.c:817 #10 0x00007f666e6eec3f in fr_cursor_next (cursor=0x7f666a584490) at src/lib/cursor.c:151 #11 0x00007f666e70b75e in pairfree (vps=0x7f666a584508) at src/lib/valuepair.c:169 #12 0x00000000004493d6 in sess_free_vps (parent=0x7f664c0b76f0, data_ptr=0x7f662c07b140, ad=0x7f664c0b78e8, idx=0, argl=0, argp=0x0) at src/main/tls.c:1986 #13 0x00000037a0c6a68a in ?? () from /usr/lib64/libcrypto.so.10 #14 0x00000037a7c466bc in SSL_SESSION_free () from /usr/lib64/libssl.so.10 #15 0x00000037a7c445cd in SSL_free () from /usr/lib64/libssl.so.10 #16 0x000000000044624f in session_close (ssn=0x7f6644063950) at src/main/tls.c:599
At a guess, this is wrong:
https://github.com/FreeRADIUS/freeradius-server/blob/v3.0.x/src/modules/rlm_...
...because FR_TLS_EX_INDEX_TALLOC is used to parent the VPs paircopy'ed in:
https://github.com/FreeRADIUS/freeradius-server/blob/v3.0.x/src/main/tls.c#L...
I spent a bit of time looking at this earlier; the lifecycle of some of these objects is really really really hard to follow, both because OpenSSL is a bit weird, and because the naming of the FR TLS stuff is used in different ways to similarly-named objects inside OpenSSL.
Summary for my own notes:
OpenSSL:
SSL_CTX* is an OpenSSL object containing the certs/keys/settings, that creates an...
SSL* is an OpenSSL "connection" i.e. an SSL handshake, that creates or uses a cached...
SSL_SESSION* is a negotiated OpenSSL master key & algos
FreeRADIUS
tls_session_t is an OpenSSL SSL* i.e. connection plus some bits i.e. the BIOs to feed into/out of OpenSS
eap_handler_t is an EAP exchange; it's keyed off State which mutates on every round trip. It contains a bunch of eap counter stuff and the tls_session_t
Given that sess_free_vps() is called by OpenSSL when the session goes away, I guess we don't need to parent the cached VPs to anything; ditto the cached certs?
Certainly don't want to parent the vps/certs to handler or tls_session_t; those have shorter lifetimes than SSL_SESSION* objects.
Guessing SSL_SESSION is the one that can be resumed? It would be nice to have a context associated with it if possible, and then we could parent that cached VPs off that... it allows easy cleanup. I don't know the libssl API well... is there away to associate a ctx pointer with a cached session, and an appropriate session destructor callback. Arran Cudbard-Bell <a.cudbardb@freeradius.org> FreeRADIUS Development Team FD31 3077 42EC 7FCD 32FE 5EE2 56CF 27F9 30A8 CAA2
On 24 Jun 2014, at 18:25, Arran Cudbard-Bell <a.cudbardb@freeradius.org> wrote:
On 24 Jun 2014, at 17:47, Phil Mayers <p.mayers@IMPERIAL.AC.UK> wrote:
(Starting a new thread to keep mailreader depth sane)
#4 0x00007f666e6ef97a in _fr_talloc_fault (reason=0x36ad408378 "Bad talloc magic value - unknown value") at src/lib/debug.c:561 #5 0x00000036ad402df1 in talloc_abort_unknown_value (ptr=<value optimized out>) at ../talloc.c:341 #6 talloc_chunk_from_ptr (ptr=<value optimized out>) at ../talloc.c:360 #7 talloc_get_name (ptr=<value optimized out>) at ../talloc.c:1153 #8 0x00000036ad4057eb in _talloc_get_type_abort (ptr=0x7f662c091dd0, name=0x7f666e714bdb "VALUE_PAIR", location=0x7f666e714bc7 "src/lib/debug.c:817") at ../talloc.c:1206 #9 0x00007f666e6f0104 in fr_verify_vp (file=0x7f666e7144f0 "src/lib/cursor.c", line=151, vp=0x7f662c091dd0) at src/lib/debug.c:817 #10 0x00007f666e6eec3f in fr_cursor_next (cursor=0x7f666a584490) at src/lib/cursor.c:151 #11 0x00007f666e70b75e in pairfree (vps=0x7f666a584508) at src/lib/valuepair.c:169 #12 0x00000000004493d6 in sess_free_vps (parent=0x7f664c0b76f0, data_ptr=0x7f662c07b140, ad=0x7f664c0b78e8, idx=0, argl=0, argp=0x0) at src/main/tls.c:1986 #13 0x00000037a0c6a68a in ?? () from /usr/lib64/libcrypto.so.10 #14 0x00000037a7c466bc in SSL_SESSION_free () from /usr/lib64/libssl.so.10 #15 0x00000037a7c445cd in SSL_free () from /usr/lib64/libssl.so.10 #16 0x000000000044624f in session_close (ssn=0x7f6644063950) at src/main/tls.c:599
At a guess, this is wrong:
https://github.com/FreeRADIUS/freeradius-server/blob/v3.0.x/src/modules/rlm_...
...because FR_TLS_EX_INDEX_TALLOC is used to parent the VPs paircopy'ed in:
https://github.com/FreeRADIUS/freeradius-server/blob/v3.0.x/src/main/tls.c#L...
I spent a bit of time looking at this earlier; the lifecycle of some of these objects is really really really hard to follow, both because OpenSSL is a bit weird, and because the naming of the FR TLS stuff is used in different ways to similarly-named objects inside OpenSSL.
Summary for my own notes:
OpenSSL:
SSL_CTX* is an OpenSSL object containing the certs/keys/settings, that creates an...
SSL* is an OpenSSL "connection" i.e. an SSL handshake, that creates or uses a cached...
SSL_SESSION* is a negotiated OpenSSL master key & algos
FreeRADIUS
tls_session_t is an OpenSSL SSL* i.e. connection plus some bits i.e. the BIOs to feed into/out of OpenSS
eap_handler_t is an EAP exchange; it's keyed off State which mutates on every round trip. It contains a bunch of eap counter stuff and the tls_session_t
Given that sess_free_vps() is called by OpenSSL when the session goes away, I guess we don't need to parent the cached VPs to anything; ditto the cached certs?
Certainly don't want to parent the vps/certs to handler or tls_session_t; those have shorter lifetimes than SSL_SESSION* objects.
Guessing SSL_SESSION is the one that can be resumed? It would be nice to have a context associated with it if possible, and then we could parent that cached VPs off that... it allows easy cleanup.
I don't know the libssl API well... is there away to associate a ctx pointer with a cached session, and an appropriate session destructor callback.
*? I also noticed with the on disk cache, the entries never seem to get cleaned up... is this intentional, is it up to administrators to add a cron job to remove very old session data? I guess that makes sense, sill slightly surprising. Arran Cudbard-Bell <a.cudbardb@freeradius.org> FreeRADIUS Development Team FD31 3077 42EC 7FCD 32FE 5EE2 56CF 27F9 30A8 CAA2
That was an intentional "phase 1 simplification"; the code updates the modtime when a session is pulled off disk and the default eap config suggests a periodic "find" command to clean the spool. I didn't see a good way of running background tasks at the time and definitely didn't like the idea of blocking the server in a request context whilst walking a potentially large directory. There's a couple of other warts; really there should be a 2- or 3-level directory structure to avoid enormous numbers of inodes in one and the separate session .asn1 and pairlist files are a bit yucky, though easy to debug. So it could be improved; if I ever get us on 3.x in production we'll be able to actually use it and I'll have a better idea how to proceed. As for attaching a context to a session yes that should be relatively easy and would actually clean up the code as only the top level context needs an openssl free function; the rest could be talloc destructors. I'll work up a patch tomorrow; I have a good idea what needs touching. -- Sent from my mobile device, please excuse brevity and typos
Oh also the tls-config construct means "cache.name" might be shared by >1 module which may have the effect of allowing e.g. a peap session to be resumed against the ttls module with crazy/bad results. There is probably some way to avoid this... -- Sent from my mobile device, please excuse brevity and typos
Phil Mayers wrote:
Oh also the tls-config construct means "cache.name" might be shared by >1 module which may have the effect of allowing e.g. a peap session to be resumed against the ttls module with crazy/bad results. There is probably some way to avoid this...
Hmm... that should probably be avoided, but I'm not sure it causes any issue. There's no protocol-specific information saved in the cache. It's just a TLS session. Alan DeKok.
Phil Mayers wrote:
At a guess, this is wrong:
https://github.com/FreeRADIUS/freeradius-server/blob/v3.0.x/src/modules/rlm_...
Yes. I've pushed a fix.
I spent a bit of time looking at this earlier; the lifecycle of some of these objects is really really really hard to follow, both because OpenSSL is a bit weird, and because the naming of the FR TLS stuff is used in different ways to similarly-named objects inside OpenSSL.
It's sad.
Given that sess_free_vps() is called by OpenSSL when the session goes away, I guess we don't need to parent the cached VPs to anything; ditto the cached certs?
It would be nice to use talloc...
Certainly don't want to parent the vps/certs to handler or tls_session_t; those have shorter lifetimes than SSL_SESSION* objects.
Yes. I've just used a NULL ctx. Alan DeKok.
It's sad.
Given that sess_free_vps() is called by OpenSSL when the session goes away, I guess we don't need to parent the cached VPs to anything; ditto the cached certs?
It would be nice to use talloc...
Certainly don't want to parent the vps/certs to handler or tls_session_t; those have shorter lifetimes than SSL_SESSION* objects.
Yes. I've just used a NULL ctx.
Phil's proposed patch would still be useful if he has time :) Arran Cudbard-Bell <a.cudbardb@freeradius.org> FreeRADIUS Development Team FD31 3077 42EC 7FCD 32FE 5EE2 56CF 27F9 30A8 CAA2
On 25/06/14 00:31, Alan DeKok wrote:
Phil Mayers wrote:
At a guess, this is wrong:
https://github.com/FreeRADIUS/freeradius-server/blob/v3.0.x/src/modules/rlm_...
Yes. I've pushed a fix.
Just for reference, #43f3f03 has run for over 24 hours under heavy load now, stable memory usage and no faults. Looking good! Now back to our regular programming (pardon the pun)...
On 26 Jun 2014, at 12:20, Alan DeKok <aland@deployingradius.com> wrote:
Phil Mayers wrote:
Just for reference, #43f3f03 has run for over 24 hours under heavy load now, stable memory usage and no faults. Looking good!
Finally... it's been a long haul, with a few surprises in undocumented talloc behavior.
Woot. Arran Cudbard-Bell <a.cudbardb@freeradius.org> FreeRADIUS Development Team FD31 3077 42EC 7FCD 32FE 5EE2 56CF 27F9 30A8 CAA2
participants (4)
-
Alan DeKok -
Alan DeKok -
Arran Cudbard-Bell -
Phil Mayers