Is this a memory leak?

[lishaobingdong]

Hello, I noticed that ASan reported a memory leak, but I'm not quite sure if it was caused by my client server, or caused by lsquic. In get_or_generate_state in lsquic_tokgen.c:198, data and key_copy are not freed before the function returns, so I think it may cause a memory leak.

[litespeedtech]

Yes, it is a memory leak, you can try the following patch

diff --git a/src/liblsquic/lsquic_stock_shi.c b/src/liblsquic/lsquic_stock_shi.c
index b6807cf8..61f36140 100644
--- a/src/liblsquic/lsquic_stock_shi.c
+++ b/src/liblsquic/lsquic_stock_shi.c
@@ -42,8 +42,6 @@ struct hash_elem
 static void
 free_key_data (struct hash_elem *he)
 {
-    if (he->data_sz)
-        free(he->data);
     free(he->key.buf);
 }
 
@@ -86,9 +84,17 @@ stock_shi_insert (void *hash_ctx, void *key, unsigned key_sz,
     he = lsquic_malo_get(hash->malo);
     if (!he)
         return -1;
-    he->key.buf = key;
+    he->key.buf = malloc(key_sz + data_sz + 1);
+    if (!he->key.buf)
+    {
+        lsquic_malo_put(he);
+        return -1;
+    }
+    memmove(he->key.buf, key, key_sz);
+    ((char *)(he->key.buf))[key_sz] = 0;
     he->key.sz  = key_sz;
-    he->data    = data;
+    he->data    = he->key.buf + he->key.sz + 1;
+    memmove(he->data, data, data_sz);
     he->data_sz = data_sz;
     he->expiry = expiry;
     memset(&he->lhash_elem, 0, sizeof(he->lhash_elem));
diff --git a/src/liblsquic/lsquic_tokgen.c b/src/liblsquic/lsquic_tokgen.c
index 93a12df8..81ca5a4d 100644
--- a/src/liblsquic/lsquic_tokgen.c
+++ b/src/liblsquic/lsquic_tokgen.c
@@ -151,7 +151,8 @@ get_or_generate_state (struct lsquic_engine_public *enpub, time_t now,
 {
     const struct lsquic_shared_hash_if *const shi = enpub->enp_shi;
     void *const ctx = enpub->enp_shi_ctx;
-    void *data, *copy, *key_copy;
+    void *data, *copy;
+    char key_copy[TOKGEN_SHM_KEY_SIZE];
     int s;
     unsigned sz;
     size_t bufsz;
@@ -236,28 +237,13 @@ get_or_generate_state (struct lsquic_engine_public *enpub, time_t now,
     memcpy(shm_state->tgss_magic_bottom, TOKGEN_SHM_MAGIC_BOTTOM,
                                         sizeof(TOKGEN_SHM_MAGIC_BOTTOM) - 1);
 
-    data = malloc(sizeof(*shm_state));
-    if (!data)
-    {
-        LSQ_ERROR("%s: malloc", __func__);
-        return -1;
-    }
-    memcpy(data, shm_state, sizeof(*shm_state));
-    key_copy = malloc(TOKGEN_SHM_KEY_SIZE);
-    if (!key_copy)
-    {
-        LSQ_ERROR("%s: malloc", __func__);
-        free(data);
-        return -1;
-    }
+    data = shm_state;
     memcpy(key_copy, TOKGEN_SHM_KEY, TOKGEN_SHM_KEY_SIZE);
     s = shi->shi_insert(ctx, key_copy, TOKGEN_SHM_KEY_SIZE, data,
                                                     sizeof(*shm_state), 0);
     if (s != 0)
     {
         LSQ_ERROR("cannot insert into SHM");
-        free(data);
-        free(key_copy);
         return -1;
     }
     sz = sizeof(*shm_state);
diff --git a/tests/test_shi.c b/tests/test_shi.c
index 8e6b9109..4c6f73f5 100644
--- a/tests/test_shi.c
+++ b/tests/test_shi.c
@@ -23,30 +23,6 @@ static const struct pair {
 };
 
 
-struct data {
-    size_t   size;      /* Overall size including the payload */
-    char    *key;
-    char    *value;
-    char     data[0];   /* key followed by value */
-};
-
-
-static struct data *
-new_data (const char *key, const char *value)
-{
-    size_t klen = strlen(key);
-    size_t vlen = strlen(value);
-    size_t size = klen + vlen + 2 + sizeof(struct data);
-    struct data *data = malloc(size);
-    data->size = size;
-    data->key = data->data;
-    data->value = data->data + klen + 1;
-    memcpy(data->data, key, klen);
-    data->key[klen] = '\0';
-    memcpy(data->value, value, vlen);
-    data->value[vlen] = '\0';
-    return data;
-}
 
 
 #define N_PAIRS (sizeof(pairs) / sizeof(pairs[0]))
@@ -72,7 +48,6 @@ test_shi (const struct order *order)
     const time_t now = time(NULL);
     time_t expiry;
     void *datap;
-    struct data *data;
 
     hash = lsquic_stock_shared_hash_new();
 
@@ -83,9 +58,8 @@ test_shi (const struct order *order)
             expiry = now + 1;
         else
             expiry = 0;
-        data = new_data(pair->key, pair->value);
-        s = stock_shi.shi_insert(hash, strdup(data->key), strlen(data->key),
-                            data, data->size, expiry);
+        s = stock_shi.shi_insert(hash, (char *)pair->key, strlen(pair->key),
+                            (char *)pair->value, strlen(pair->value), expiry);
         assert(0 == s);
     }
 
@@ -106,11 +80,9 @@ test_shi (const struct order *order)
         }
         else
         {
-            data = datap;
             assert(1 == s);
-            assert(data_sz == data->size);
-            assert(0 == strcmp(pair->key, data->key));
-            assert(0 == strcmp(pair->value, data->value));
+            assert(data_sz == strlen(pair->value));
+            assert(0 == strncmp(pair->value, datap, data_sz));
         }
     }
 

[litespeedtech]

It is in release v3.1.2 already, assume it has been fixed.