From 7e2cea3d26701ff1d80fda7d8ca907890e3e7baa Mon Sep 17 00:00:00 2001 From: Dave Barach Date: Wed, 9 Oct 2019 12:57:13 -0400 Subject: vppinfra: fix page boundary crossing bug in hash_memory64 Fix a day-1 bug, possibly dating back as far as 2002. The zap64() game involves fetching 8 byte chunks, and clearing octets not to be included in the key. That's fine *unless* the 8-byte fetch happens to cross a page boundary into unmapped or no-access space. Type: fix Signed-off-by: Dave Barach Change-Id: I4607e9840032257c96ba7387f86c931c0921749d --- src/vppinfra/hash.c | 51 +++++++++++++++++++++++++++++++++++++++++++++++---- 1 file changed, 47 insertions(+), 4 deletions(-) (limited to 'src/vppinfra') diff --git a/src/vppinfra/hash.c b/src/vppinfra/hash.c index eae79d48592..b6f0901dd68 100644 --- a/src/vppinfra/hash.c +++ b/src/vppinfra/hash.c @@ -103,14 +103,32 @@ zap64 (u64 x, word n) * Therefore all the 8 Bytes of the u64 are systematically read, which * rightfully causes address-sanitizer to raise an error on smaller inputs. * - * However the invalid Bytes are discarded within zap64(), whicj is why + * However the invalid Bytes are discarded within zap64(), which is why * this can be silenced safely. + * + * The above is true *unless* the extra bytes cross a page boundary + * into unmapped or no-access space, hence the boundary crossing check. */ static inline u64 __attribute__ ((no_sanitize_address)) hash_memory64 (void *p, word n_bytes, u64 state) { u64 *q = p; u64 a, b, c, n; + int page_boundary_crossing; + u64 start_addr, end_addr; + union + { + u8 as_u8[8]; + u64 as_u64; + } tmp; + + /* + * If the request crosses a 4k boundary, it's not OK to assume + * that the zap64 game is safe. 4k is the minimum known page size. + */ + start_addr = (u64) p; + end_addr = start_addr + n_bytes + 7; + page_boundary_crossing = (start_addr >> 12) != (end_addr >> 12); a = b = 0x9e3779b97f4a7c13LL; c = state; @@ -133,18 +151,43 @@ hash_memory64 (void *p, word n_bytes, u64 state) a += clib_mem_unaligned (q + 0, u64); b += clib_mem_unaligned (q + 1, u64); if (n % sizeof (u64)) - c += zap64 (clib_mem_unaligned (q + 2, u64), n % sizeof (u64)) << 8; + { + if (PREDICT_TRUE (page_boundary_crossing == 0)) + c += + zap64 (clib_mem_unaligned (q + 2, u64), n % sizeof (u64)) << 8; + else + { + clib_memcpy_fast (tmp.as_u8, q + 2, n % sizeof (u64)); + c += zap64 (tmp.as_u64, n % sizeof (u64)) << 8; + } + } break; case 1: a += clib_mem_unaligned (q + 0, u64); if (n % sizeof (u64)) - b += zap64 (clib_mem_unaligned (q + 1, u64), n % sizeof (u64)); + { + if (PREDICT_TRUE (page_boundary_crossing == 0)) + b += zap64 (clib_mem_unaligned (q + 1, u64), n % sizeof (u64)); + else + { + clib_memcpy_fast (tmp.as_u8, q + 1, n % sizeof (u64)); + b += zap64 (tmp.as_u64, n % sizeof (u64)); + } + } break; case 0: if (n % sizeof (u64)) - a += zap64 (clib_mem_unaligned (q + 0, u64), n % sizeof (u64)); + { + if (PREDICT_TRUE (page_boundary_crossing == 0)) + a += zap64 (clib_mem_unaligned (q + 0, u64), n % sizeof (u64)); + else + { + clib_memcpy_fast (tmp.as_u8, q, n % sizeof (u64)); + a += zap64 (tmp.as_u64, n % sizeof (u64)); + } + } break; } -- cgit 1.2.3-korg