summaryrefslogtreecommitdiffstats
path: root/src/vppinfra/hash.c
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/vppinfra/hash.c
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/vppinfra/hash.c')
-rw-r--r--src/vppinfra/hash.c18
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)