Hi Arne, That method you listed is not the one that I had captured in my GDB backtrace. The potentially problematic method is the key_state_free() and there are indeed free_buff() calls in that one which could be operating on a NULL pointer if its zero'd out still which could be bad! If you could take a look at this method instead:
https://github.com/OpenVPN/openvpn/blob/master/src/openvpn/ssl.c#L916 There is a tls_session_init() method which calls key_state_init() on only the KS_PRIMARY key and not the KS_LAME_DUCK key as you can see below! https://github.com/OpenVPN/openvpn/blob/master/src/openvpn/ssl.c#L1032 This is not good to do because in the tls_session_free() method, it is calling key_state_free() on ALL keys regardless without checking any of them before!! https://github.com/OpenVPN/openvpn/blob/master/src/openvpn/ssl.c#L1066 There shouldn't be index mismatches for init's and free's in general for good quality coding practices and I believe this is the code path that my debugging session hit when doing my testing which I considered a bug and have worked around in my own branches. Are you able to confirm the details that I have mentioned above? Thanks for your time, Jon C On Mon, Nov 3, 2025 at 5:12 AM Arne Schwabe <[email protected]> wrote: > Am 02.11.25 um 20:52 schrieb Jon Chiappetta via Openvpn-devel: > > Hi Gert and devs, > > > > During my experimentation with this dual mode, I've had to go through a > > lot of the code in the ssl.c library file. While I was tracing and > > testing my modifications, I think I found another potential crash path > > but I'm also not sure if it was my modifications or if this is indeed a > > bug potentially. > > > > This was the GDB backtrace I was able to capture during my own modified > > testing: > > > > > > > > Thread 8 "openvpn" received signal SIGSEGV, Segmentation fault. > > [Switching to Thread 0x7fffdffff6c0 (LWP 638500)] > > 0x00007ffff74add55 in __GI___libc_free (mem=0x2) at > ./malloc/malloc.c:3375 > > warning: 3375 ./malloc/malloc.c: No such file or directory > > (gdb) Quit > > (gdb) bt > > #0 0x00007ffff74add55 in __GI___libc_free (mem=0x2) at ./malloc/ > > malloc.c:3375 > > #1 0x0000555555564be1 in free_buf (buf=buf@entry=0x7fffd40068b0) at > > buffer.c:186 > > #2 0x00005555555e1d04 in key_state_free (ks=ks@entry=0x7fffd40065d0, > clear=clear@entry=false) at ssl.c:928 > > > void > key_state_ssl_free(struct key_state_ssl *ks_ssl) > { > if (ks_ssl->ssl) > { > #ifdef BIO_DEBUG > bio_debug_oc("close ssl_bio", ks_ssl->ssl_bio); > bio_debug_oc("close ct_in", ks_ssl->ct_in); > bio_debug_oc("close ct_out", ks_ssl->ct_out); > #endif > BIO_free_all(ks_ssl->ssl_bio); > SSL_free(ks_ssl->ssl); > } > } > > This method does not have a free_buf that is in your backtrace. So it > looks more like it is only in your modified version. > > In general you have to a bit careful. Often the code assumes that the > struct are initlised with zero, which also makes pointers and so on > NULL. And the original method here is also in this pattern and checking > if (ks_ssl->ssl) before doing anything. The pattern that free is used on > something that is not initialised (but only is all zero) is a quite a > common pattern in OpenVPN. > > Arne >
_______________________________________________ Openvpn-devel mailing list [email protected] https://lists.sourceforge.net/lists/listinfo/openvpn-devel
