stats: missing dimension in stat_set_simple_counter
A simple counter is a two dimensional array by threads and
counter index. 28017 introduced an error missing the first
dimension.
If a vector is updated at the same time as a client reads,
an invalid pointer my result. This will be caught by the
optimistic locking after copying out the data, but if
following a pointer outside of the stat segment then
the stat client would crash. Add suitable boundary checks
for access to stat memory segment.
Fixes: 7d29e320fb2855a1ddb7a6af09078b8ed636de01
Type: fix
Signed-off-by: Ole Troan <ot@cisco.com>
Change-Id: I94f124ec71d98218c4eda5d124ac5594743d93d6
diff --git a/src/vpp-api/client/stat_client.c b/src/vpp-api/client/stat_client.c
index 56ff387..018cce3 100644
--- a/src/vpp-api/client/stat_client.c
+++ b/src/vpp-api/client/stat_client.c
@@ -192,6 +192,16 @@
return stat_segment_heartbeat_r (sm);
}
+#define stat_vec_dup(S,V) \
+ ({ \
+ __typeof__ ((V)[0]) * _v(v) = 0; \
+ if (V && ((void *)V > (void *)S->shared_header) && \
+ (((void*)V + vec_bytes(V)) < \
+ ((void *)S->shared_header + S->memory_size))) \
+ _v(v) = vec_dup(V); \
+ _v(v); \
+})
+
static stat_segment_data_t
copy_data (stat_segment_directory_entry_t * ep, stat_client_main_t * sm)
{
@@ -213,21 +223,21 @@
case STAT_DIR_TYPE_COUNTER_VECTOR_SIMPLE:
simple_c = stat_segment_adjust (sm, ep->data);
- result.simple_counter_vec = vec_dup (simple_c);
+ result.simple_counter_vec = stat_vec_dup (sm, simple_c);
for (i = 0; i < vec_len (simple_c); i++)
{
counter_t *cb = stat_segment_adjust (sm, simple_c[i]);
- result.simple_counter_vec[i] = vec_dup (cb);
+ result.simple_counter_vec[i] = stat_vec_dup (sm, cb);
}
break;
case STAT_DIR_TYPE_COUNTER_VECTOR_COMBINED:
combined_c = stat_segment_adjust (sm, ep->data);
- result.combined_counter_vec = vec_dup (combined_c);
+ result.combined_counter_vec = stat_vec_dup (sm, combined_c);
for (i = 0; i < vec_len (combined_c); i++)
{
vlib_counter_t *cb = stat_segment_adjust (sm, combined_c[i]);
- result.combined_counter_vec[i] = vec_dup (cb);
+ result.combined_counter_vec[i] = stat_vec_dup (sm, cb);
}
break;
@@ -246,11 +256,11 @@
case STAT_DIR_TYPE_NAME_VECTOR:
{
uint8_t **name_vector = stat_segment_adjust (sm, ep->data);
- result.name_vector = vec_dup (name_vector);
+ result.name_vector = stat_vec_dup (sm, name_vector);
for (i = 0; i < vec_len (name_vector); i++)
{
u8 *name = stat_segment_adjust (sm, name_vector[i]);
- result.name_vector[i] = vec_dup (name);
+ result.name_vector[i] = stat_vec_dup (sm, name);
}
}
break;
@@ -290,6 +300,8 @@
case STAT_DIR_TYPE_ERROR_INDEX:
vec_free (res[i].error_vector);
break;
+ case STAT_DIR_TYPE_SCALAR_INDEX:
+ break;
default:
assert (0);
}
diff --git a/src/vpp-api/client/stat_client.h b/src/vpp-api/client/stat_client.h
index c5fa559..f8473ef 100644
--- a/src/vpp-api/client/stat_client.h
+++ b/src/vpp-api/client/stat_client.h
@@ -101,8 +101,12 @@
static inline void *
stat_segment_adjust (stat_client_main_t * sm, void *data)
{
- return (void *) ((char *) sm->shared_header +
- ((char *) data - (char *) sm->shared_header->base));
+ void *p = (void *) ((char *) sm->shared_header +
+ ((char *) data - (char *) sm->shared_header->base));
+ if (p > (void *) sm->shared_header &&
+ ((p + sizeof (p)) < ((void *) sm->shared_header + sm->memory_size)))
+ return p;
+ return 0;
}
/*
diff --git a/src/vpp/stats/stat_segment.c b/src/vpp/stats/stat_segment.c
index b060969..4244acb 100644
--- a/src/vpp/stats/stat_segment.c
+++ b/src/vpp/stats/stat_segment.c
@@ -281,7 +281,8 @@
u32 thread_index, u32 index, u64 value)
{
ASSERT (ep->data);
- counter_t *cb = ep->data;
+ counter_t **counters = ep->data;
+ counter_t *cb = counters[thread_index];
cb[index] = value;
}