From: Hugo Landau <hlandau@openssl.org>
Date: Mon, 21 Nov 2022 07:55:37 +0000 (+0000)
Subject: QUIC Front-End I/O API: Ensure BIOs are reffed and freed correctly
X-Git-Tag: openssl-3.2.0-alpha1~1490
X-Git-Url: http://git.ipfire.org/cgi-bin/gitweb.cgi?a=commitdiff_plain;h=d1ac77b1a50b070aa55384f4c5eff3df71adb2c7;p=thirdparty%2Fopenssl.git

QUIC Front-End I/O API: Ensure BIOs are reffed and freed correctly

Reviewed-by: Tomas Mraz <tomas@openssl.org>
Reviewed-by: Matt Caswell <matt@openssl.org>
(Merged from https://github.com/openssl/openssl/pull/19703)
---

diff --git a/include/internal/quic_channel.h b/include/internal/quic_channel.h
index 452c39af479..9c1adfd667a 100644
--- a/include/internal/quic_channel.h
+++ b/include/internal/quic_channel.h
@@ -141,8 +141,8 @@ int ossl_quic_channel_set_peer_addr(QUIC_CHANNEL *ch, const BIO_ADDR *peer_addr)
 /* Gets/sets the underlying network read and write BIOs. */
 BIO *ossl_quic_channel_get_net_rbio(QUIC_CHANNEL *ch);
 BIO *ossl_quic_channel_get_net_wbio(QUIC_CHANNEL *ch);
-int ossl_quic_channel_set0_net_rbio(QUIC_CHANNEL *ch, BIO *net_rbio);
-int ossl_quic_channel_set0_net_wbio(QUIC_CHANNEL *ch, BIO *net_wbio);
+int ossl_quic_channel_set_net_rbio(QUIC_CHANNEL *ch, BIO *net_rbio);
+int ossl_quic_channel_set_net_wbio(QUIC_CHANNEL *ch, BIO *net_wbio);
 
 /*
  * Returns an existing stream by stream ID. Returns NULL if the stream does not
diff --git a/ssl/quic/quic_channel.c b/ssl/quic/quic_channel.c
index 04f87c92925..431344bcf16 100644
--- a/ssl/quic/quic_channel.c
+++ b/ssl/quic/quic_channel.c
@@ -283,8 +283,6 @@ static void ch_cleanup(QUIC_CHANNEL *ch)
     ossl_qrx_free(ch->qrx);
     ossl_quic_demux_free(ch->demux);
     OPENSSL_free(ch->local_transport_params);
-    BIO_free(ch->net_rbio);
-    BIO_free(ch->net_wbio);
 }
 
 QUIC_CHANNEL *ossl_quic_channel_new(const QUIC_CHANNEL_ARGS *args)
@@ -1283,7 +1281,13 @@ BIO *ossl_quic_channel_get_net_wbio(QUIC_CHANNEL *ch)
     return ch->net_wbio;
 }
 
-int ossl_quic_channel_set0_net_rbio(QUIC_CHANNEL *ch, BIO *net_rbio)
+/*
+ * QUIC_CHANNEL does not ref any BIO it is provided with, nor is any ref
+ * transferred to it. The caller (i.e., QUIC_CONNECTION) is responsible for
+ * ensuring the BIO lasts until the channel is freed or the BIO is switched out
+ * for another BIO by a subsequent successful call to this function.
+ */
+int ossl_quic_channel_set_net_rbio(QUIC_CHANNEL *ch, BIO *net_rbio)
 {
     BIO_POLL_DESCRIPTOR d = {0};
 
@@ -1300,13 +1304,12 @@ int ossl_quic_channel_set0_net_rbio(QUIC_CHANNEL *ch, BIO *net_rbio)
     }
 
     ossl_quic_reactor_set_poll_r(&ch->rtor, &d);
-    BIO_free(ch->net_rbio);
     ossl_quic_demux_set_bio(ch->demux, net_rbio);
     ch->net_rbio = net_rbio;
     return 1;
 }
 
-int ossl_quic_channel_set0_net_wbio(QUIC_CHANNEL *ch, BIO *net_wbio)
+int ossl_quic_channel_set_net_wbio(QUIC_CHANNEL *ch, BIO *net_wbio)
 {
     BIO_POLL_DESCRIPTOR d = {0};
 
@@ -1323,7 +1326,6 @@ int ossl_quic_channel_set0_net_wbio(QUIC_CHANNEL *ch, BIO *net_wbio)
     }
 
     ossl_quic_reactor_set_poll_w(&ch->rtor, &d);
-    BIO_free(ch->net_wbio);
     ossl_qtx_set_bio(ch->qtx, net_wbio);
     ch->net_wbio = net_wbio;
     return 1;
diff --git a/ssl/quic/quic_impl.c b/ssl/quic/quic_impl.c
index b6df5f778b6..6d78990cecf 100644
--- a/ssl/quic/quic_impl.c
+++ b/ssl/quic/quic_impl.c
@@ -152,6 +152,9 @@ void ossl_quic_free(SSL *s)
 
     ossl_quic_channel_free(qc->ch);
 
+    BIO_free(qc->net_rbio);
+    BIO_free(qc->net_wbio);
+
     /* Note: SSL_free calls OPENSSL_free(qc) for us */
 }
 
@@ -271,9 +274,10 @@ void ossl_quic_conn_set0_net_rbio(QUIC_CONNECTION *qc, BIO *net_rbio)
     if (qc->net_rbio == net_rbio)
         return;
 
-    if (qc->ch != NULL && !ossl_quic_channel_set0_net_rbio(qc->ch, net_rbio))
+    if (qc->ch != NULL && !ossl_quic_channel_set_net_rbio(qc->ch, net_rbio))
         return;
 
+    BIO_free(qc->net_rbio);
     qc->net_rbio = net_rbio;
 
     /*
@@ -298,9 +302,10 @@ void ossl_quic_conn_set0_net_wbio(QUIC_CONNECTION *qc, BIO *net_wbio)
     if (qc->net_wbio == net_wbio)
         return;
 
-    if (qc->ch != NULL && !ossl_quic_channel_set0_net_wbio(qc->ch, net_wbio))
+    if (qc->ch != NULL && !ossl_quic_channel_set_net_wbio(qc->ch, net_wbio))
         return;
 
+    BIO_free(qc->net_wbio);
     qc->net_wbio = net_wbio;
 
     if (net_wbio != NULL) {
@@ -564,8 +569,8 @@ static int configure_channel(QUIC_CONNECTION *qc)
 {
     assert(qc->ch != NULL);
 
-    if (!ossl_quic_channel_set0_net_rbio(qc->ch, qc->net_rbio)
-        || !ossl_quic_channel_set0_net_wbio(qc->ch, qc->net_wbio)
+    if (!ossl_quic_channel_set_net_rbio(qc->ch, qc->net_rbio)
+        || !ossl_quic_channel_set_net_wbio(qc->ch, qc->net_wbio)
         || !ossl_quic_channel_set_peer_addr(qc->ch, &qc->init_peer_addr))
         return 0;
 
@@ -614,8 +619,6 @@ int ossl_quic_do_handshake(QUIC_CONNECTION *qc)
 {
     int ret;
 
-    fprintf(stderr, "# handshake begin\n");
-
     if (qc->ch != NULL && ossl_quic_channel_is_term_any(qc->ch))
         return QUIC_RAISE_NON_NORMAL_ERROR(qc, SSL_R_PROTOCOL_IS_SHUTDOWN, NULL);