stats: fix use-after-free hash key string

Hash keys are not copied by the hash infrastructure, instead the pointer
is used directly. stat_segment_register_gauge() does not allocate a
private object for the key, causing issues when it is freed or reused.
Allocate a private object on insertion into the hashtable instead.

Type: fix
Fixes: 92e3082199d10add866894e86a9762d79a3536c4

Change-Id: Ifb6addfcaec81bdb7ea3512050ce55f06ef09a4c
Signed-off-by: Benoît Ganne <bganne@cisco.com>

diff --git a/src/vlib/error.c b/src/vlib/error.c
index ef50663..58e1343 100644 (file)
--- a/src/vlib/error.c
+++ b/src/vlib/error.c
@@ -160,15 +160,18 @@ vlib_register_errors (vlib_main_t * vm,
   /* Register counter indices in the stat segment directory */
   {
     int i;
-    u8 *error_name;
+    u8 *error_name = 0;
 
     for (i = 0; i < n_errors; i++)
       {
-       error_name = format (0, "/err/%v/%s%c", n->name, error_strings[i], 0);
-       /* Note: error_name consumed by the following call */
+       vec_reset_length (error_name);
+       error_name =
+         format (error_name, "/err/%v/%s%c", n->name, error_strings[i], 0);
        vlib_stats_register_error_index (oldheap, error_name, em->counters,
                                         n->error_heap_index + i);
       }
+
+    vec_free (error_name);
   }
 
   /* (re)register the em->counters base address, switch back to main heap */

diff --git a/src/vpp/stats/stat_segment.c b/src/vpp/stats/stat_segment.c
index ec0bcf9..1328ea8 100644 (file)
--- a/src/vpp/stats/stat_segment.c
+++ b/src/vpp/stats/stat_segment.c
@@ -67,12 +67,14 @@ lookup_or_create_hash_index (u8 * name, u32 next_vector_index)
   hash_pair_t *hp;
 
   /* Must be called in the context of the main heap */
-  ASSERT (clib_mem_get_heap != sm->heap);
+  ASSERT (clib_mem_get_heap () != sm->heap);
 
   hp = hash_get_pair (sm->directory_vector_by_name, name);
   if (!hp)
     {
-      hash_set (sm->directory_vector_by_name, name, next_vector_index);
+      /* we allocate our private copy of 'name' */
+      hash_set (sm->directory_vector_by_name, format (0, "%s%c", name, 0),
+               next_vector_index);
       index = next_vector_index;
     }
   else
@@ -188,10 +190,6 @@ vlib_stats_register_error_index (void *oldheap, u8 * name, u64 * em_vec,
       shared_header->directory_offset =
        stat_segment_offset (shared_header, sm->directory_vector);
     }
-  else
-    {
-      vec_free (name);
-    }
 
   vlib_stat_segment_unlock ();
 }