summaryrefslogtreecommitdiffstats
path: root/src
diff options
context:
space:
mode:
authorBenoît Ganne <bganne@cisco.com>2021-02-11 19:46:43 +0100
committerDamjan Marion <dmarion@me.com>2021-02-15 16:17:14 +0000
commit1a3e08a7197addb1c07e66c1b1da3286c9bcb140 (patch)
treefcbb03afc8aa57d3c9c63a7cdf5a27d0c24cf502 /src
parentce3f8249b59d3c3540cf0b87cc1c2f0d3a3a1814 (diff)
vppinfra: fix memcpy undefined behaviour
Calling mem{cpy,move} with NULL pointers results in undefined behaviour. This in turns is exploited by GCC. For example, the sequence: memcpy (dst, src, n); if (!src) return; src[0] = 0xcafe; will be optimized as memcpy (dst, src, n); src[0] = 0xcafe; IOW the test for NULL is gone. vec_*() functions sometime call memcpy with NULL pointers and 0 length, triggering this optimization. For example, the sequence: vec_append(v1, v2); len = vec_len(v2); will crash if v2 is NULL, because the test for NULL pointer in vec_len() has been optimized out. This commit fixes occurrences of such undefined behaviour, and also introduces a memcpy wrapper to catch those in debug mode. Type: fix Change-Id: I175e2dd726a883f97cf7de3b15f66d4b237ddefd Signed-off-by: Benoît Ganne <bganne@cisco.com>
Diffstat (limited to 'src')
-rw-r--r--src/vppinfra/hash.c18
-rw-r--r--src/vppinfra/memcpy_avx2.h2
-rw-r--r--src/vppinfra/memcpy_avx512.h2
-rw-r--r--src/vppinfra/memcpy_sse3.h2
-rw-r--r--src/vppinfra/string.h27
-rw-r--r--src/vppinfra/vec.h152
6 files changed, 123 insertions, 80 deletions
diff --git a/src/vppinfra/hash.c b/src/vppinfra/hash.c
index 220c16989de..da37b6e88a7 100644
--- a/src/vppinfra/hash.c
+++ b/src/vppinfra/hash.c
@@ -548,6 +548,7 @@ lookup (void *v, uword key, enum lookup_opcode op,
hash_t *h = hash_header (v);
hash_pair_union_t *p = 0;
uword found_key = 0;
+ uword value_bytes;
uword i;
if (!v)
@@ -555,6 +556,7 @@ lookup (void *v, uword key, enum lookup_opcode op,
i = key_sum (h, key) & (_vec_len (v) - 1);
p = get_pair (v, i);
+ value_bytes = hash_value_bytes (h);
if (hash_is_user (v, i))
{
@@ -564,9 +566,8 @@ lookup (void *v, uword key, enum lookup_opcode op,
if (op == UNSET)
{
set_is_user (v, i, 0);
- if (old_value)
- clib_memcpy_fast (old_value, p->direct.value,
- hash_value_bytes (h));
+ if (old_value && value_bytes)
+ clib_memcpy_fast (old_value, p->direct.value, value_bytes);
zero_pair (h, &p->direct);
}
}
@@ -598,9 +599,8 @@ lookup (void *v, uword key, enum lookup_opcode op,
found_key = p != 0;
if (found_key && op == UNSET)
{
- if (old_value)
- clib_memcpy_fast (old_value, &p->direct.value,
- hash_value_bytes (h));
+ if (old_value && value_bytes)
+ clib_memcpy_fast (old_value, &p->direct.value, value_bytes);
unset_indirect (v, i, &p->direct);
@@ -611,12 +611,12 @@ lookup (void *v, uword key, enum lookup_opcode op,
}
}
- if (op == SET && p != 0)
+ if (op == SET && p != 0 && value_bytes)
{
/* Save away old value for caller. */
if (old_value && found_key)
- clib_memcpy_fast (old_value, &p->direct.value, hash_value_bytes (h));
- clib_memcpy_fast (&p->direct.value, new_value, hash_value_bytes (h));
+ clib_memcpy_fast (old_value, &p->direct.value, value_bytes);
+ clib_memcpy_fast (&p->direct.value, new_value, value_bytes);
}
if (op == SET)
diff --git a/src/vppinfra/memcpy_avx2.h b/src/vppinfra/memcpy_avx2.h
index f3f0d3f1943..f7a36f0e5cb 100644
--- a/src/vppinfra/memcpy_avx2.h
+++ b/src/vppinfra/memcpy_avx2.h
@@ -114,7 +114,7 @@ clib_mov128blocks (u8 * dst, const u8 * src, size_t n)
}
static inline void *
-clib_memcpy_fast (void *dst, const void *src, size_t n)
+clib_memcpy_fast_avx2 (void *dst, const void *src, size_t n)
{
uword dstu = (uword) dst;
uword srcu = (uword) src;
diff --git a/src/vppinfra/memcpy_avx512.h b/src/vppinfra/memcpy_avx512.h
index 1444c271ff1..98dac75beac 100644
--- a/src/vppinfra/memcpy_avx512.h
+++ b/src/vppinfra/memcpy_avx512.h
@@ -144,7 +144,7 @@ clib_mov512blocks (u8 * dst, const u8 * src, size_t n)
}
static inline void *
-clib_memcpy_fast (void *dst, const void *src, size_t n)
+clib_memcpy_fast_avx512 (void *dst, const void *src, size_t n)
{
uword dstu = (uword) dst;
uword srcu = (uword) src;
diff --git a/src/vppinfra/memcpy_sse3.h b/src/vppinfra/memcpy_sse3.h
index d9e4ac668e9..aea2005d95a 100644
--- a/src/vppinfra/memcpy_sse3.h
+++ b/src/vppinfra/memcpy_sse3.h
@@ -188,7 +188,7 @@ clib_mov256 (u8 * dst, const u8 * src)
})
static inline void *
-clib_memcpy_fast (void *dst, const void *src, size_t n)
+clib_memcpy_fast_sse3 (void *dst, const void *src, size_t n)
{
__m128i xmm0, xmm1, xmm2, xmm3, xmm4, xmm5, xmm6, xmm7, xmm8;
uword dstu = (uword) dst;
diff --git a/src/vppinfra/string.h b/src/vppinfra/string.h
index 4f96450ce9e..fb46a0c6a0d 100644
--- a/src/vppinfra/string.h
+++ b/src/vppinfra/string.h
@@ -46,6 +46,7 @@
#include <vppinfra/clib.h> /* for CLIB_LINUX_KERNEL */
#include <vppinfra/vector.h>
+#include <vppinfra/error_bootstrap.h>
#ifdef CLIB_LINUX_KERNEL
#include <linux/string.h>
@@ -73,16 +74,30 @@ void clib_memswap (void *_a, void *_b, uword bytes);
#ifndef __COVERITY__
#if __AVX512BITALG__
#include <vppinfra/memcpy_avx512.h>
+#define clib_memcpy_fast_arch(a, b, c) clib_memcpy_fast_avx512 (a, b, c)
#elif __AVX2__
#include <vppinfra/memcpy_avx2.h>
+#define clib_memcpy_fast_arch(a, b, c) clib_memcpy_fast_avx2 (a, b, c)
#elif __SSSE3__
#include <vppinfra/memcpy_sse3.h>
-#else
-#define clib_memcpy_fast(a,b,c) memcpy(a,b,c)
-#endif
-#else /* __COVERITY__ */
-#define clib_memcpy_fast(a,b,c) memcpy(a,b,c)
-#endif
+#define clib_memcpy_fast_arch(a, b, c) clib_memcpy_fast_sse3 (a, b, c)
+#endif /* __AVX512BITALG__ */
+#endif /* __COVERITY__ */
+
+#ifndef clib_memcpy_fast_arch
+#define clib_memcpy_fast_arch(a, b, c) memcpy (a, b, c)
+#endif /* clib_memcpy_fast_arch */
+
+static_always_inline void *
+clib_memcpy_fast (void *restrict dst, const void *restrict src, size_t n)
+{
+ ASSERT (dst && src &&
+ "memcpy(src, dst, n) with src == NULL or dst == NULL is undefined "
+ "behaviour");
+ return clib_memcpy_fast_arch (dst, src, n);
+}
+
+#undef clib_memcpy_fast_arch
/* c-11 string manipulation variants */
diff --git a/src/vppinfra/vec.h b/src/vppinfra/vec.h
index e8146af7098..d19ff998137 100644
--- a/src/vppinfra/vec.h
+++ b/src/vppinfra/vec.h
@@ -665,13 +665,19 @@ do { \
@param A alignment (may be zero)
@return V (value-result macro parameter)
*/
-#define vec_add_ha(V,E,N,H,A) \
-do { \
- word _v(n) = (N); \
- word _v(l) = vec_len (V); \
- V = _vec_resize ((V), _v(n), (_v(l) + _v(n)) * sizeof ((V)[0]), (H), (A)); \
- clib_memcpy_fast ((V) + _v(l), (E), _v(n) * sizeof ((V)[0])); \
-} while (0)
+#define vec_add_ha(V, E, N, H, A) \
+ do \
+ { \
+ word _v (n) = (N); \
+ if (PREDICT_TRUE (_v (n) > 0)) \
+ { \
+ word _v (l) = vec_len (V); \
+ V = _vec_resize ((V), _v (n), (_v (l) + _v (n)) * sizeof ((V)[0]), \
+ (H), (A)); \
+ clib_memcpy_fast ((V) + _v (l), (E), _v (n) * sizeof ((V)[0])); \
+ } \
+ } \
+ while (0)
/** \brief Add N elements to end of vector V (no header, unspecified alignment)
@@ -819,22 +825,23 @@ do { \
@return V (value-result macro parameter)
*/
-#define vec_insert_elts_ha(V,E,N,M,H,A) \
-do { \
- word _v(l) = vec_len (V); \
- word _v(n) = (N); \
- word _v(m) = (M); \
- V = _vec_resize ((V), \
- _v(n), \
- (_v(l) + _v(n))*sizeof((V)[0]), \
- (H), (A)); \
- ASSERT (_v(m) <= _v(l)); \
- memmove ((V) + _v(m) + _v(n), \
- (V) + _v(m), \
- (_v(l) - _v(m)) * sizeof ((V)[0])); \
- clib_memcpy_fast ((V) + _v(m), (E), \
- _v(n) * sizeof ((V)[0])); \
-} while (0)
+#define vec_insert_elts_ha(V, E, N, M, H, A) \
+ do \
+ { \
+ word _v (n) = (N); \
+ if (PREDICT_TRUE (_v (n) > 0)) \
+ { \
+ word _v (l) = vec_len (V); \
+ word _v (m) = (M); \
+ V = _vec_resize ((V), _v (n), (_v (l) + _v (n)) * sizeof ((V)[0]), \
+ (H), (A)); \
+ ASSERT (_v (m) <= _v (l)); \
+ memmove ((V) + _v (m) + _v (n), (V) + _v (m), \
+ (_v (l) - _v (m)) * sizeof ((V)[0])); \
+ clib_memcpy_fast ((V) + _v (m), (E), _v (n) * sizeof ((V)[0])); \
+ } \
+ } \
+ while (0)
/** \brief Insert N vector elements starting at element M,
insert given elements (no header, unspecified alignment)
@@ -902,15 +909,21 @@ do { \
@param V2 vector to append
*/
-#define vec_append(v1,v2) \
-do { \
- uword _v(l1) = vec_len (v1); \
- uword _v(l2) = vec_len (v2); \
- \
- v1 = _vec_resize ((v1), _v(l2), \
- (_v(l1) + _v(l2)) * sizeof ((v1)[0]), 0, 0); \
- clib_memcpy_fast ((v1) + _v(l1), (v2), _v(l2) * sizeof ((v2)[0])); \
-} while (0)
+#define vec_append(v1, v2) \
+ do \
+ { \
+ uword _v (l1) = vec_len (v1); \
+ uword _v (l2) = vec_len (v2); \
+ \
+ if (PREDICT_TRUE (_v (l2) > 0)) \
+ { \
+ v1 = _vec_resize ((v1), _v (l2), \
+ (_v (l1) + _v (l2)) * sizeof ((v1)[0]), 0, 0); \
+ clib_memcpy_fast ((v1) + _v (l1), (v2), \
+ _v (l2) * sizeof ((v2)[0])); \
+ } \
+ } \
+ while (0)
/** \brief Append v2 after v1. Result in v1. Specified alignment.
@param V1 target vector
@@ -918,31 +931,42 @@ do { \
@param align required alignment
*/
-#define vec_append_aligned(v1,v2,align) \
-do { \
- uword _v(l1) = vec_len (v1); \
- uword _v(l2) = vec_len (v2); \
- \
- v1 = _vec_resize ((v1), _v(l2), \
- (_v(l1) + _v(l2)) * sizeof ((v1)[0]), 0, align); \
- clib_memcpy_fast ((v1) + _v(l1), (v2), _v(l2) * sizeof ((v2)[0])); \
-} while (0)
+#define vec_append_aligned(v1, v2, align) \
+ do \
+ { \
+ uword _v (l1) = vec_len (v1); \
+ uword _v (l2) = vec_len (v2); \
+ \
+ if (PREDICT_TRUE (_v (l2) > 0)) \
+ { \
+ v1 = _vec_resize ( \
+ (v1), _v (l2), (_v (l1) + _v (l2)) * sizeof ((v1)[0]), 0, align); \
+ clib_memcpy_fast ((v1) + _v (l1), (v2), \
+ _v (l2) * sizeof ((v2)[0])); \
+ } \
+ } \
+ while (0)
/** \brief Prepend v2 before v1. Result in v1.
@param V1 target vector
@param V2 vector to prepend
*/
-#define vec_prepend(v1,v2) \
-do { \
- uword _v(l1) = vec_len (v1); \
- uword _v(l2) = vec_len (v2); \
- \
- v1 = _vec_resize ((v1), _v(l2), \
- (_v(l1) + _v(l2)) * sizeof ((v1)[0]), 0, 0); \
- memmove ((v1) + _v(l2), (v1), _v(l1) * sizeof ((v1)[0])); \
- clib_memcpy_fast ((v1), (v2), _v(l2) * sizeof ((v2)[0])); \
-} while (0)
+#define vec_prepend(v1, v2) \
+ do \
+ { \
+ uword _v (l1) = vec_len (v1); \
+ uword _v (l2) = vec_len (v2); \
+ \
+ if (PREDICT_TRUE (_v (l2) > 0)) \
+ { \
+ v1 = _vec_resize ((v1), _v (l2), \
+ (_v (l1) + _v (l2)) * sizeof ((v1)[0]), 0, 0); \
+ memmove ((v1) + _v (l2), (v1), _v (l1) * sizeof ((v1)[0])); \
+ clib_memcpy_fast ((v1), (v2), _v (l2) * sizeof ((v2)[0])); \
+ } \
+ } \
+ while (0)
/** \brief Prepend v2 before v1. Result in v1. Specified alignment
@param V1 target vector
@@ -950,17 +974,21 @@ do { \
@param align required alignment
*/
-#define vec_prepend_aligned(v1,v2,align) \
-do { \
- uword _v(l1) = vec_len (v1); \
- uword _v(l2) = vec_len (v2); \
- \
- v1 = _vec_resize ((v1), _v(l2), \
- (_v(l1) + _v(l2)) * sizeof ((v1)[0]), 0, align); \
- memmove ((v1) + _v(l2), (v1), _v(l1) * sizeof ((v1)[0])); \
- clib_memcpy_fast ((v1), (v2), _v(l2) * sizeof ((v2)[0])); \
-} while (0)
-
+#define vec_prepend_aligned(v1, v2, align) \
+ do \
+ { \
+ uword _v (l1) = vec_len (v1); \
+ uword _v (l2) = vec_len (v2); \
+ \
+ if (PREDICT_TRUE (_v (l2) > 0)) \
+ { \
+ v1 = _vec_resize ( \
+ (v1), _v (l2), (_v (l1) + _v (l2)) * sizeof ((v1)[0]), 0, align); \
+ memmove ((v1) + _v (l2), (v1), _v (l1) * sizeof ((v1)[0])); \
+ clib_memcpy_fast ((v1), (v2), _v (l2) * sizeof ((v2)[0])); \
+ } \
+ } \
+ while (0)
/** \brief Zero all vector elements. Null-pointer tolerant.
@param var Vector to zero