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 452c39af479195faeadc1840dc87d557ffccb2f7..9c1adfd667a4594ecac45f2a71cecc59d01378be 100644 (file)
--- 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 04f87c92925eee7317bd7f413d45d247b7f3b351..431344bcf16f7ad45d76020bcdc9549ae2ad4d69 100644 (file)
--- 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 b6df5f778b65fec6e71e861eb08ebd0a5d51c967..6d78990cecf28782712b048b4be9d41f73cb20fa 100644 (file)
--- 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);