stats: fix memory leakage when adding / deleting interfaces 77/20277/3
authorOle Troan <ot@cisco.com>
Fri, 21 Jun 2019 14:07:06 +0000 (16:07 +0200)
committerDave Wallace <dwallacelf@gmail.com>
Fri, 21 Jun 2019 16:06:19 +0000 (16:06 +0000)
This fixes two leaks in registering errors in the stats segment.
- The error name created by vlib_register_errors() was not freed.
- Duplicate error names (when interface readded) was added to the vector.

Change-Id: If5fe371e8059cf6678fc785cbf673707b4f4a655
Type: fix
Signed-off-by: Ole Troan <ot@cisco.com>
src/vlib/error.c
src/vpp/stats/stat_segment.c

index 1a48c9e..17447b0 100644 (file)
@@ -109,10 +109,11 @@ vlib_error_drop_buffers (vlib_main_t * vm,
   return n_buffers;
 }
 
-void vlib_stats_register_error_index (u8 *, u64 *, u64)
+void vlib_stats_register_error_index (void *, u8 *, u64 *, u64)
   __attribute__ ((weak));
 void
-vlib_stats_register_error_index (u8 * notused, u64 * notused2, u64 notused3)
+vlib_stats_register_error_index (void *noutused, u8 * notused2,
+                                u64 * notused3, u64 notused4)
 {
 };
 
@@ -180,7 +181,7 @@ vlib_register_errors (vlib_main_t * vm,
       {
        error_name = format (0, "/err/%v/%s%c", n->name, error_strings[i], 0);
        /* Note: error_name consumed by the following call */
-       vlib_stats_register_error_index (error_name, em->counters,
+       vlib_stats_register_error_index (oldheap, error_name, em->counters,
                                         n->error_heap_index + i);
       }
   }
index b9d5b23..442a750 100644 (file)
@@ -152,7 +152,8 @@ vlib_stats_pop_heap (void *cm_arg, void *oldheap, u32 cindex,
 }
 
 void
-vlib_stats_register_error_index (u8 * name, u64 * em_vec, u64 index)
+vlib_stats_register_error_index (void *oldheap, u8 * name, u64 * em_vec,
+                                u64 index)
 {
   stat_segment_main_t *sm = &stat_segment_main;
   stat_segment_shared_header_t *shared_header = sm->shared_header;
@@ -162,17 +163,32 @@ vlib_stats_register_error_index (u8 * name, u64 * em_vec, u64 index)
   ASSERT (shared_header);
 
   vlib_stat_segment_lock ();
+  u32 next_vector_index = vec_len (sm->directory_vector);
+  clib_mem_set_heap (oldheap); /* Exit stats segment */
 
-  memcpy (e.name, name, vec_len (name));
-  e.name[vec_len (name)] = '\0';
-  e.type = STAT_DIR_TYPE_ERROR_INDEX;
-  e.offset = index;
-  e.offset_vector = 0;
-  vec_add1 (sm->directory_vector, e);
+  u32 vector_index = lookup_or_create_hash_index (oldheap, (char *) name,
+                                                 next_vector_index);
 
-  /* Warn clients to refresh any pointers they might be holding */
-  shared_header->directory_offset =
-    stat_segment_offset (shared_header, sm->directory_vector);
+  /* Back to stats segment */
+  clib_mem_set_heap (sm->heap);        /* Re-enter stat segment */
+
+  if (next_vector_index == vector_index)
+    {
+      memcpy (e.name, name, vec_len (name));
+      e.name[vec_len (name)] = '\0';
+      e.type = STAT_DIR_TYPE_ERROR_INDEX;
+      e.offset = index;
+      e.offset_vector = 0;
+      vec_add1 (sm->directory_vector, e);
+
+      /* Warn clients to refresh any pointers they might be holding */
+      shared_header->directory_offset =
+       stat_segment_offset (shared_header, sm->directory_vector);
+    }
+  else
+    {
+      vec_free (name);
+    }
 
   vlib_stat_segment_unlock ();
 }