Re: [Openvpn-devel] One more program crash that I happened to observe and catch in GDB - potential free/malloc bug?

[Jon C. <ro...@fo...>]

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 <ar...@rf...> 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
>