aboutsummaryrefslogtreecommitdiffstats
path: root/src/vppinfra/hash.c
diff options
context:
space:
mode:
authorDave Barach <dave@barachs.net>2019-10-09 12:57:13 -0400
committerDamjan Marion <dmarion@me.com>2019-10-11 12:30:58 +0000
commit7e2cea3d26701ff1d80fda7d8ca907890e3e7baa (patch)
tree49dfbace1a56e09ef46402573090f9880fa57f2f /src/vppinfra/hash.c
parent8563cb389a7e8d6d4e042e146c0d94b8af98ca7a (diff)
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 <dave@barachs.net> Change-Id: I4607e9840032257c96ba7387f86c931c0921749d
Diffstat (limited to 'src/vppinfra/hash.c')
-rw-r--r--src/vppinfra/hash.c51
1 files changed, 47 insertions, 4 deletions
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;
}