[tor-commits] [tor/master] Fix CID 1442277

[nickm at torproject.org]

commit 1bcececdd8e2ff4d6e921270984ba7800c914f64
Author: rl1987 <rl1987 at sdf.lonestar.org>
Date:   Sat Jan 5 11:01:07 2019 +0200

    Fix CID 1442277
---
 changes/bug28989           |  5 +++++
 src/test/test_hs_service.c | 32 ++++++++++++++++++++++++++++++++
 2 files changed, 37 insertions(+)

diff --git a/changes/bug28989 b/changes/bug28989
new file mode 100644
index 000000000..3e3ccccaf
--- /dev/null
+++ b/changes/bug28989
@@ -0,0 +1,5 @@
+  o Minor bugfixes (unit tests):
+    - Instead of relying on hs_free_all() to clean up all onion service
+      objects we created in test_build_descriptors(), deallocate
+      them one by one. This lets Coverity know that we are not leaking memory
+      here and fixes CID 1442277. Fixes bug 28989; bugfix on 0.3.5.1-alpha.
diff --git a/src/test/test_hs_service.c b/src/test/test_hs_service.c
index b2aafc1cd..3357ca2c5 100644
--- a/src/test/test_hs_service.c
+++ b/src/test/test_hs_service.c
@@ -289,6 +289,20 @@ helper_create_service(void)
   return service;
 }
 
+/* Helper: Deallocate a given service object, its child objects and
+ * remove it from onion service map.
+ * */
+static void
+helper_destroy_service(hs_service_t *service)
+{
+  if (!service)
+    return;
+
+  remove_service(get_hs_service_map(), service);
+
+  hs_service_free(service);
+}
+
 /* Helper: Return a newly allocated service object with clients. */
 static hs_service_t *
 helper_create_service_with_clients(int num_clients)
@@ -1626,6 +1640,7 @@ test_build_descriptors(void *arg)
 {
   int ret;
   time_t now = time(NULL);
+  hs_service_t *last_service = NULL;
 
   (void) arg;
 
@@ -1650,6 +1665,7 @@ test_build_descriptors(void *arg)
    * is disabled. */
   {
     hs_service_t *service = helper_create_service();
+    last_service = service;
     service_descriptor_free(service->desc_current);
     service->desc_current = NULL;
 
@@ -1660,12 +1676,16 @@ test_build_descriptors(void *arg)
     hs_desc_superencrypted_data_t *superencrypted;
     superencrypted = &service->desc_current->desc->superencrypted_data;
     tt_int_op(smartlist_len(superencrypted->clients), OP_EQ, 16);
+
+    helper_destroy_service(service);
+    last_service = NULL;
   }
 
   /* Generate a valid number of fake auth clients when the number of
    * clients is zero. */
   {
     hs_service_t *service = helper_create_service_with_clients(0);
+    last_service = service;
     service_descriptor_free(service->desc_current);
     service->desc_current = NULL;
 
@@ -1673,12 +1693,16 @@ test_build_descriptors(void *arg)
     hs_desc_superencrypted_data_t *superencrypted;
     superencrypted = &service->desc_current->desc->superencrypted_data;
     tt_int_op(smartlist_len(superencrypted->clients), OP_EQ, 16);
+
+    helper_destroy_service(service);
+    last_service = NULL;
   }
 
   /* Generate a valid number of fake auth clients when the number of
    * clients is not a multiple of 16. */
   {
     hs_service_t *service = helper_create_service_with_clients(20);
+    last_service = service;
     service_descriptor_free(service->desc_current);
     service->desc_current = NULL;
 
@@ -1686,12 +1710,16 @@ test_build_descriptors(void *arg)
     hs_desc_superencrypted_data_t *superencrypted;
     superencrypted = &service->desc_current->desc->superencrypted_data;
     tt_int_op(smartlist_len(superencrypted->clients), OP_EQ, 32);
+
+    helper_destroy_service(service);
+    last_service = NULL;
   }
 
   /* Do not generate any fake desc client when the number of clients is
    * a multiple of 16 but not zero. */
   {
     hs_service_t *service = helper_create_service_with_clients(32);
+    last_service = service;
     service_descriptor_free(service->desc_current);
     service->desc_current = NULL;
 
@@ -1699,9 +1727,13 @@ test_build_descriptors(void *arg)
     hs_desc_superencrypted_data_t *superencrypted;
     superencrypted = &service->desc_current->desc->superencrypted_data;
     tt_int_op(smartlist_len(superencrypted->clients), OP_EQ, 32);
+
+    helper_destroy_service(service);
+    last_service = NULL;
   }
 
  done:
+  helper_destroy_service(last_service);
   hs_free_all();
 }