Re: [PATCH] libressl: Fix a reference counting bug

Yes, the patch has the leak problem you point out.
I think it is more simple and clear to hold two references if s->rbio
== s->wbio.

I reworked the patch. I'm new to this project. Thank you for your patience.

=========================================================
diff --git a/src/lib/libssl/ssl_lib.c b/src/lib/libssl/ssl_lib.c
index 64988f8b0..8936104fd 100644
--- a/src/lib/libssl/ssl_lib.c
+++ b/src/lib/libssl/ssl_lib.c
@@ -545,8 +545,7 @@ SSL_free(SSL *s)
                s->bbio = NULL;
        }

-       if (s->rbio != s->wbio)
-               BIO_free_all(s->rbio);
+       BIO_free_all(s->rbio);
        BIO_free_all(s->wbio);

        tls13_ctx_free(s->tls13);
@@ -610,6 +609,9 @@ LSSL_ALIAS(SSL_up_ref);
 void
 SSL_set_bio(SSL *s, BIO *rbio, BIO *wbio)
 {
+       if (rbio == s->rbio && wbio == s->wbio)
+               return;
+
        /* If the output buffering BIO is still in place, remove it */
        if (s->bbio != NULL) {
                if (s->wbio == s->bbio) {
@@ -618,12 +620,14 @@ SSL_set_bio(SSL *s, BIO *rbio, BIO *wbio)
                }
        }

-       if (s->rbio != rbio && s->rbio != s->wbio)
+       if (s->rbio != rbio)
                BIO_free_all(s->rbio);
-       if (s->wbio != wbio && s->rbio != s->wbio)
+       if (s->wbio != wbio)
                BIO_free_all(s->wbio);
        s->rbio = rbio;
        s->wbio = wbio;
+       if (rbio != NULL && rbio == wbio)
+               BIO_up_ref(rbio);
 }
 LSSL_ALIAS(SSL_set_bio);


On Thu, 28 May 2026 at 14:32, Otto Moerbeek <[email protected]> wrote:
>
> On Thu, May 28, 2026 at 02:04:52PM +0800, Solar Flare wrote:
>
> > Hi,
> >
> > This bug causes double free of the ssl->rbio object. A sample code to
> > reproduce the issue:
> >
> > #include <openssl/ssl.h>
> > int main() {
> >     SSL* s = SSL_new(SSL_CTX_new(TLS_client_method()));
> >     SSL_set_fd(s, 1);
> >     SSL_set_bio(s, SSL_get_rbio(s), NULL);
> >     SSL_free(s); /* segfault here */
> >     return 0;
> > }
> >
> > ---------------------------------------------------------------------------
> > diff --git a/src/lib/libssl/ssl_lib.c b/src/lib/libssl/ssl_lib.c
> > index 8cce44603..64988f8b0 100644
> > --- a/src/lib/libssl/ssl_lib.c
> > +++ b/src/lib/libssl/ssl_lib.c
> > @@ -620,7 +620,7 @@ SSL_set_bio(SSL *s, BIO *rbio, BIO *wbio)
> >
> >         if (s->rbio != rbio && s->rbio != s->wbio)
> >                 BIO_free_all(s->rbio);
> > -       if (s->wbio != wbio)
> > +       if (s->wbio != wbio && s->rbio != s->wbio)
> >                 BIO_free_all(s->wbio);
> >         s->rbio = rbio;
> >         s->wbio = wbio;
> >
>
> This leaks if s->rbio == s->wbio, in that case a sinlgle call to
> BIO_free_all() shoud be done. One way to achieve that is to set s->rbio
> to NULL in the first if block.
>
>         -Otto
>
>