BUG/MINOR: quic: Possible leak when allocating an encryption level

This bug was reported by GH #2200 (coverity issue) as follows:

*** CID 1516590:  Resource leaks  (RESOURCE_LEAK)
/src/quic_tls.c: 159 in quic_conn_enc_level_init()
153
154             LIST_APPEND(&qc->qel_list, &qel->list);
155             *el = qel;
156             ret = 1;
157      leave:
158             TRACE_LEAVE(QUIC_EV_CONN_CLOSE, qc);
>>>     CID 1516590:  Resource leaks  (RESOURCE_LEAK)
>>>     Variable "qel" going out of scope leaks the storage it points to.
159             return ret;
160     }
161
162     /* Uninitialize <qel> QUIC encryption level. Never fails. */
163     void quic_conn_enc_level_uninit(struct quic_conn *qc, struct quic_enc_level *qel)
164     {

This bug was introduced by this commit which has foolishly assumed the encryption
level memory would be released after quic_conn_enc_level_init() has failed. This
is no more possible because this object is dynamic and no more a static member
of the QUIC connection object.

Anyway, this patch modifies quic_conn_enc_level_init() to ensure this is
no more leak when quic_conn_enc_level_init() fails calling quic_conn_enc_level_uninit()
in case of memory allocation error.

quic_conn_enc_level_uninit() code was moved without modification only to be defined
before quic_conn_enc_level_init()

There is no need to backport this.

diff --git a/src/quic_tls.c b/src/quic_tls.c
index cb4c37fb249d9e1396dcbe51384a10029e6699a0..f80e486e6ace1583ee1d8e5e719de31dc8e1d656 100644 (file)
--- a/src/quic_tls.c
+++ b/src/quic_tls.c
@@ -100,6 +100,25 @@ void quic_tls_secret_hexdump(struct buffer *buf,
                chunk_appendf(buf, "%02x", secret[i]);
 }
 
+/* Uninitialize <qel> QUIC encryption level. Never fails. */
+void quic_conn_enc_level_uninit(struct quic_conn *qc, struct quic_enc_level *qel)
+{
+       int i;
+
+       TRACE_ENTER(QUIC_EV_CONN_CLOSE, qc);
+
+       for (i = 0; i < qel->tx.crypto.nb_buf; i++) {
+               if (qel->tx.crypto.bufs[i]) {
+                       pool_free(pool_head_quic_crypto_buf, qel->tx.crypto.bufs[i]);
+                       qel->tx.crypto.bufs[i] = NULL;
+               }
+       }
+       ha_free(&qel->tx.crypto.bufs);
+       quic_cstream_free(qel->cstream);
+
+       TRACE_LEAVE(QUIC_EV_CONN_CLOSE, qc);
+}
+
 /* Initialize QUIC TLS encryption level with <level<> as level for <qc> QUIC
  * connection allocating everything needed.
  *
@@ -120,6 +139,9 @@ static int quic_conn_enc_level_init(struct quic_conn *qc,
        if (!qel)
                goto leave;
 
+       qel->tx.crypto.bufs = NULL;
+       qel->tx.crypto.nb_buf = 0;
+       qel->cstream = NULL;
        qel->pktns = pktns;
        qel->level = level;
        quic_tls_ctx_reset(&qel->tls_ctx);
@@ -131,11 +153,12 @@ static int quic_conn_enc_level_init(struct quic_conn *qc,
        /* TODO: use a pool */
        qel->tx.crypto.bufs = malloc(sizeof *qel->tx.crypto.bufs);
        if (!qel->tx.crypto.bufs)
-               goto leave;
+               goto err;
 
        qel->tx.crypto.bufs[0] = pool_alloc(pool_head_quic_crypto_buf);
        if (!qel->tx.crypto.bufs[0])
-               goto leave;
+               goto err;
+
 
        qel->tx.crypto.bufs[0]->sz = 0;
        qel->tx.crypto.nb_buf = 1;
@@ -148,7 +171,7 @@ static int quic_conn_enc_level_init(struct quic_conn *qc,
        else {
                qel->cstream = quic_cstream_new(qc);
                if (!qel->cstream)
-                       goto leave;
+                       goto err;
        }
 
        LIST_APPEND(&qc->qel_list, &qel->list);
@@ -157,25 +180,11 @@ static int quic_conn_enc_level_init(struct quic_conn *qc,
  leave:
        TRACE_LEAVE(QUIC_EV_CONN_CLOSE, qc);
        return ret;
-}
 
-/* Uninitialize <qel> QUIC encryption level. Never fails. */
-void quic_conn_enc_level_uninit(struct quic_conn *qc, struct quic_enc_level *qel)
-{
-       int i;
-
-       TRACE_ENTER(QUIC_EV_CONN_CLOSE, qc);
-
-       for (i = 0; i < qel->tx.crypto.nb_buf; i++) {
-               if (qel->tx.crypto.bufs[i]) {
-                       pool_free(pool_head_quic_crypto_buf, qel->tx.crypto.bufs[i]);
-                       qel->tx.crypto.bufs[i] = NULL;
-               }
-       }
-       ha_free(&qel->tx.crypto.bufs);
-       quic_cstream_free(qel->cstream);
-
-       TRACE_LEAVE(QUIC_EV_CONN_CLOSE, qc);
+ err:
+       quic_conn_enc_level_uninit(qc, qel);
+       pool_free(pool_head_quic_enc_level, qel);
+       goto leave;
 }
 
 /* Allocate a QUIC TLS encryption with <level> as TLS stack encryption to be