Commit 92b5ba86f6 for openssl.org
commit 92b5ba86f68b200c0f2549ec21a5edaaf2da4735
Author: Joshua Rogers <MegaManSec@users.noreply.github.com>
Date: Sun Oct 12 21:30:50 2025 +0800
quic: remove redundant free of inner TLS in accept_connection
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)
diff --git a/ssl/quic/quic_impl.c b/ssl/quic/quic_impl.c
index 46a34a1063..04004980db 100644
--- a/ssl/quic/quic_impl.c
+++ b/ssl/quic/quic_impl.c
@@ -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;