]> git.ipfire.org Git - thirdparty/openssl.git/commitdiff
quic: remove redundant free of inner TLS in accept_connection
authorJoshua Rogers <MegaManSec@users.noreply.github.com>
Sun, 12 Oct 2025 13:30:50 +0000 (21:30 +0800)
committerTomas Mraz <tomas@openssl.org>
Mon, 12 Jan 2026 18:51:35 +0000 (19:51 +0100)
SSL_free(conn_ssl) for a QCSO enters ossl_quic_free, which calls qc_cleanup.
qc_cleanup already frees qc->tls via SSL_free(qc->tls) and then frees qc->ch.
The additional SSL_free(ossl_quic_channel_get0_tls(new_ch)) releases the same
TLS a second time, which is redundant.

We also replace some of the pure condition checks with ossl_assert() checks
as these conditions cannot really fail.

Signed-off-by: Joshua Rogers <MegaManSec@users.noreply.github.com>
Reviewed-by: Saša Nedvědický <sashan@openssl.org>
Reviewed-by: Tim Hudson <tjh@openssl.org>
Reviewed-by: Tomas Mraz <tomas@openssl.org>
MergeDate: Mon Jan 12 18:54:07 2026
(Merged from https://github.com/openssl/openssl/pull/28920)

ssl/quic/quic_impl.c

index 46a34a10631579f65bcbd1df62b312bba9036a95..04004980dbda1de95b6fb6d3992243c6645fe7c4 100644 (file)
@@ -4785,25 +4785,34 @@ SSL *ossl_quic_accept_connection(SSL *ssl, uint64_t flags)
      * created channel, so once we pop the new channel from the port above
      * we just need to extract it
      */
-    if (new_ch == NULL
-        || (conn_ssl = ossl_quic_channel_get0_tls(new_ch)) == NULL
-        || (conn = SSL_CONNECTION_FROM_SSL(conn_ssl)) == NULL
-        || (conn_ssl = SSL_CONNECTION_GET_USER_SSL(conn)) == NULL)
+    if (new_ch == NULL)
         goto out;
+
+    /*
+     * All objects below must exist, because new_ch != NULL. The objects are
+     * bound to new_ch. If channel constructor fails to create any item here
+     * it just fails to create channel.
+     */
+    if (!ossl_assert((conn_ssl = ossl_quic_channel_get0_tls(new_ch)) != NULL)
+        || !ossl_assert((conn = SSL_CONNECTION_FROM_SSL(conn_ssl)) != NULL)
+        || !ossl_assert((conn_ssl = SSL_CONNECTION_GET_USER_SSL(conn)) != NULL))
+        goto out;
+
     qc = (QUIC_CONNECTION *)conn_ssl;
-    qc->listener = ctx.ql;
     qc->pending = 0;
     if (!SSL_up_ref(&ctx.ql->obj.ssl)) {
+        /*
+         * You might expect ossl_quic_channel_free() to be called here. Be
+         * assured it happens, The process goes as follows:
+         *    - The SSL_free() here is being handled by ossl_quic_free().
+         *    - The very last step of ossl_quic_free() is call to qc_cleanup()
+         *      where channel gets freed.
+         */
         SSL_free(conn_ssl);
-        SSL_free(ossl_quic_channel_get0_tls(new_ch));
-        conn_ssl = NULL;
     }
+    qc->listener = ctx.ql;
 
 out:
-    if (conn_ssl == NULL && new_ch != NULL) {
-        ossl_quic_channel_free(new_ch);
-        new_ch = NULL;
-    }
 
     qctx_unlock(&ctx);
     return conn_ssl;