Commit 16892155e1 for openssl.org
commit 16892155e15340ba9725d01cd9593726376fd995
Author: Nikolas Gauder <nikolas.gauder@tum.de>
Date: Tue Mar 17 20:29:28 2026 +0100
quic: fix NULL txl dereference in qtx_resize_txe
Fixes: 1957148384c7 "QUIC Record Layer (Refactor and TX Side)"
Reviewed-by: Saša Nedvědický <sashan@openssl.org>
Reviewed-by: Paul Dale <paul.dale@oracle.com>
Reviewed-by: Eugene Syromiatnikov <esyr@openssl.org>
MergeDate: Sat Apr 11 20:55:10 2026
(Merged from https://github.com/openssl/openssl/pull/30474)
diff --git a/ssl/quic/quic_record_tx.c b/ssl/quic/quic_record_tx.c
index 8714437232..0dbdb1491a 100644
--- a/ssl/quic/quic_record_tx.c
+++ b/ssl/quic/quic_record_tx.c
@@ -269,9 +269,9 @@ static TXE *qtx_ensure_free_txe(OSSL_QTX *qtx, size_t alloc_len)
* of the TXE might change; the new address is returned, or NULL on failure, in
* which case the original TXE remains valid.
*/
-static TXE *qtx_resize_txe(OSSL_QTX *qtx, TXE_LIST *txl, TXE *txe, size_t n)
+static TXE *qtx_resize_txe(OSSL_QTX *qtx, TXE *txe, size_t n)
{
- TXE *txe2, *p;
+ TXE *txe2;
/* Should never happen. */
if (txe == NULL)
@@ -280,27 +280,21 @@ static TXE *qtx_resize_txe(OSSL_QTX *qtx, TXE_LIST *txl, TXE *txe, size_t n)
if (n >= SIZE_MAX - sizeof(TXE))
return NULL;
- /* Remove the item from the list to avoid accessing freed memory */
- p = ossl_list_txe_prev(txe);
- ossl_list_txe_remove(txl, txe);
+ /*
+ * to resize txe, the caller must detach it from the list first,
+ * fail if txe is still attached.
+ */
+ if (!ossl_assert(ossl_list_txe_prev(txe) == NULL
+ && ossl_list_txe_next(txe) == NULL))
+ return NULL;
/*
* NOTE: We do not clear old memory, although it does contain decrypted
* data.
*/
txe2 = OPENSSL_realloc(txe, sizeof(TXE) + n);
- if (txe2 == NULL) {
- if (p == NULL)
- ossl_list_txe_insert_head(txl, txe);
- else
- ossl_list_txe_insert_after(txl, p, txe);
+ if (txe2 == NULL)
return NULL;
- }
-
- if (p == NULL)
- ossl_list_txe_insert_head(txl, txe2);
- else
- ossl_list_txe_insert_after(txl, p, txe2);
if (qtx->cons == txe)
qtx->cons = txe2;
@@ -313,13 +307,12 @@ static TXE *qtx_resize_txe(OSSL_QTX *qtx, TXE_LIST *txl, TXE *txe, size_t n)
* Ensure the data buffer attached to an TXE is at least n bytes in size.
* Returns NULL on failure.
*/
-static TXE *qtx_reserve_txe(OSSL_QTX *qtx, TXE_LIST *txl,
- TXE *txe, size_t n)
+static TXE *qtx_reserve_txe(OSSL_QTX *qtx, TXE *txe, size_t n)
{
if (txe->alloc_len >= n)
return txe;
- return qtx_resize_txe(qtx, txl, txe, n);
+ return qtx_resize_txe(qtx, txe, n);
}
/* Move a TXE from pending to free. */
@@ -840,6 +833,16 @@ int ossl_qtx_write_pkt(OSSL_QTX *qtx, const OSSL_QTX_PKT *pkt)
* serialize/encrypt the packet. We always encrypt packets as soon as
* our caller gives them to us, which relieves the caller of any need to
* keep the plaintext around.
+ *
+ * the txe can have three distinct states:
+ * - attached to free list
+ * - attached to tx list
+ * - detached.
+ *
+ * if txe is detached (not member of free/tx list), then it is kept
+ * in qtx->cons. The qtx_ensure_cons() here either returns the txe
+ * from free list or existing ->cons txe. The txe we obtain here
+ * is detached.
*/
txe = qtx_ensure_cons(qtx);
if (txe == NULL)
@@ -849,9 +852,20 @@ int ossl_qtx_write_pkt(OSSL_QTX *qtx, const OSSL_QTX_PKT *pkt)
* Ensure TXE has at least MDPL bytes allocated. This should only be
* possible if the MDPL has increased.
*/
- txe = qtx_reserve_txe(qtx, NULL, txe, qtx->mdpl);
- if (txe == NULL)
+ txe = qtx_reserve_txe(qtx, txe, qtx->mdpl);
+ if (txe == NULL) {
+ /*
+ * realloc of txe failed. however it is still kept in ->cons,
+ * no memory leak.
+ * The question is what we should do here to handle error,
+ * is doing `return 0` enough? or shall we discard ->cons and
+ * put it back to free list?
+ * or just stop coalescing the packet and dispatch it to network
+ * right now so the next packet tx can start from fresh?
+ * I think this is the problem for another day.
+ */
return 0;
+ }
if (!was_coalescing) {
/* Set addresses in TXE. */
@@ -878,6 +892,11 @@ int ossl_qtx_write_pkt(OSSL_QTX *qtx, const OSSL_QTX_PKT *pkt)
/*
* We failed due to insufficient length, so end the current
* datagram and try again.
+ *
+ * the ossl_qtx_finish_dgram() also puts the txe (-.cons) to
+ * tx list, so ->cons becomes attached again. The function also
+ * sets ->cons to NULL so the next loop iteration starts with
+ * fresh txe (which is also safe to resize).
*/
ossl_qtx_finish_dgram(qtx);
was_coalescing = 0;
diff --git a/test/quicapitest.c b/test/quicapitest.c
index 84a5c9566f..0d79ceb7f2 100644
--- a/test/quicapitest.c
+++ b/test/quicapitest.c
@@ -19,6 +19,7 @@
#include "testutil.h"
#include "testutil/output.h"
#include "../ssl/ssl_local.h"
+#include "../ssl/quic/quic_channel_local.h"
#include "internal/quic_error.h"
static OSSL_LIB_CTX *libctx = NULL;
@@ -3517,6 +3518,73 @@ err:
#endif
}
+static int test_quic_resize_txe(void)
+{
+ SSL_CTX *cctx = NULL;
+ SSL *clientquic = NULL;
+ QUIC_TSERVER *qtserv = NULL;
+ QUIC_CHANNEL *ch = NULL;
+ unsigned char msg[] = "resize test";
+ unsigned char buf[sizeof(msg)];
+ size_t numbytes = 0;
+ int ret = 0;
+
+ if (!TEST_ptr(cctx = SSL_CTX_new_ex(libctx, NULL, OSSL_QUIC_client_method())))
+ goto end;
+
+ if (!TEST_true(qtest_create_quic_objects(libctx, cctx, NULL,
+ cert, privkey, 0,
+ &qtserv, &clientquic,
+ NULL, NULL)))
+ goto end;
+
+ if (!TEST_true(qtest_create_quic_connection(qtserv, clientquic)))
+ goto end;
+
+ /*
+ * Client writes first to open stream 0 (client-initiated bidirectional).
+ * The server must see the stream before it can write back on it.
+ */
+ if (!TEST_true(SSL_write_ex(clientquic, msg, sizeof(msg), &numbytes))
+ || !TEST_size_t_eq(numbytes, sizeof(msg)))
+ goto end;
+
+ ossl_quic_tserver_tick(qtserv);
+ if (!TEST_true(ossl_quic_tserver_read(qtserv, 0, buf, sizeof(buf),
+ &numbytes)))
+ goto end;
+
+ /*
+ * Increase the server's QTX MDPL above the initial allocation size
+ * (QUIC_MIN_INITIAL_DGRAM_LEN = 1200). All TXEs in the free list have
+ * alloc_len = 1200, so the next write will trigger qtx_resize_txe.
+ */
+ ch = ossl_quic_tserver_get_channel(qtserv);
+ if (!TEST_true(ossl_qtx_set_mdpl(ch->qtx,
+ QUIC_MIN_INITIAL_DGRAM_LEN + 250)))
+ goto end;
+
+ /* Trigger a server write: exercises qtx_resize_txe via qtx_reserve_txe */
+ if (!TEST_true(ossl_quic_tserver_write(qtserv, 0,
+ msg, sizeof(msg), &numbytes))
+ || !TEST_size_t_eq(numbytes, sizeof(msg)))
+ goto end;
+
+ ossl_quic_tserver_tick(qtserv);
+ SSL_handle_events(clientquic);
+
+ if (!TEST_true(SSL_read_ex(clientquic, buf, sizeof(buf), &numbytes))
+ || !TEST_mem_eq(buf, numbytes, msg, sizeof(msg)))
+ goto end;
+
+ ret = 1;
+end:
+ ossl_quic_tserver_free(qtserv);
+ SSL_free(clientquic);
+ SSL_CTX_free(cctx);
+ return ret;
+}
+
/***********************************************************************************/
OPT_TEST_DECLARE_USAGE("provider config certsdir datadir\n")
@@ -3625,6 +3693,7 @@ int setup_tests(void)
ADD_TEST(test_quic_peer_addr_v6);
ADD_TEST(test_quic_peer_addr_v4);
ADD_TEST(test_ech);
+ ADD_TEST(test_quic_resize_txe);
return 1;
err: