summaryrefslogtreecommitdiffstats
diff options
context:
space:
mode:
authorOle Troan <ot@cisco.com>2020-10-21 11:55:28 +0200
committerMatthew Smith <mgsmith@netgate.com>2020-10-21 18:58:58 +0000
commit65c56c83ce4e58178b5ad90a8f325692c9904381 (patch)
tree606128e07913c4221bb1e867cf39aa06b80238b1
parent7ff514b32c8b41366b408acf2c535298546c6d42 (diff)
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
-rw-r--r--src/vpp-api/client/stat_client.c24
-rw-r--r--src/vpp-api/client/stat_client.h8
-rw-r--r--src/vpp/stats/stat_segment.c3
3 files changed, 26 insertions, 9 deletions
diff --git a/src/vpp-api/client/stat_client.c b/src/vpp-api/client/stat_client.c
index 56ff387d343..018cce34228 100644
--- a/src/vpp-api/client/stat_client.c
+++ b/src/vpp-api/client/stat_client.c
@@ -192,6 +192,16 @@ stat_segment_heartbeat (void)
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 @@ copy_data (stat_segment_directory_entry_t * ep, stat_client_main_t * sm)
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 @@ copy_data (stat_segment_directory_entry_t * ep, stat_client_main_t * sm)
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 @@ stat_segment_data_free (stat_segment_data_t * res)
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 c5fa5597758..f8473efd47f 100644
--- a/src/vpp-api/client/stat_client.h
+++ b/src/vpp-api/client/stat_client.h
@@ -101,8 +101,12 @@ _time_now_nsec (void)
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 b060969f04f..4244acb1dd0 100644
--- a/src/vpp/stats/stat_segment.c
+++ b/src/vpp/stats/stat_segment.c
@@ -281,7 +281,8 @@ stat_set_simple_counter (stat_segment_directory_entry_t * ep,
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;
}