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>

1 files changed, 9 insertions, 9 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)