From 1a3e08a7197addb1c07e66c1b1da3286c9bcb140 Mon Sep 17 00:00:00 2001 From: Benoît Ganne Date: Thu, 11 Feb 2021 19:46:43 +0100 Subject: vppinfra: fix memcpy undefined behaviour MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 --- src/vppinfra/hash.c | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) (limited to 'src/vppinfra/hash.c') 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) -- cgit 1.2.3-korg