From da868755cafcbba7eb50f87ca3449286296aa112 Mon Sep 17 00:00:00 2001 From: Andrew Tridgell Date: Mon, 3 Aug 2026 08:43:06 +1000 Subject: [PATCH] testsuite: run the SIMD checksums against an unreadable page simdtest allocated 64 spare bytes so it could test an unaligned buffer, which is exactly the slack that hid a 64-byte over-read in the AVX2 assembly for as long as it existed. Add a pass that places the buffer flush against a PROT_NONE page, so a read past the end faults in the test rather than in somebody's transfer. Every length from 128 to 4096, so each remainder mod 64 and both alignments are covered, and all four implementations are checked -- the assembly was the one at fault here, but the intrinsic paths preload too. It fails closed. A guard this test cannot set up means it is not testing what the caller thinks, so a failed sysconf/mmap/mprotect is a failure rather than a pass that looks identical to a real one. And because the dispatcher falls back on a CPU without AVX2 -- where the guard loop proves nothing about the code under test -- it says which of the two happened rather than letting a fallback run read as coverage. Without the fix this segfaults; the suite's simd-checksum test reports the non-zero exit. (cherry picked from commit 63fcfa399e249119aea09b376d1128773b804e43) --- simd-checksum-x86_64.cpp | 66 ++++++++++++++++++++++++++++++++++++++++ 1 file changed, 66 insertions(+) diff --git a/simd-checksum-x86_64.cpp b/simd-checksum-x86_64.cpp index 81910bba..b25a6922 100644 --- a/simd-checksum-x86_64.cpp +++ b/simd-checksum-x86_64.cpp @@ -548,6 +548,8 @@ int main() { #endif /* BENCHMARK_SIMD_CHECKSUM1 */ #ifdef TEST_SIMD_CHECKSUM1 +#include +#include static uint32 checksum_via_default(char *buf, int32 len) { @@ -587,6 +589,68 @@ static uint32 checksum_via_avx2(char *buf, int32 len) return (s1 & 0xffff) + (s2 << 16); } + +/* Run every implementation on a buffer placed flush against an unreadable page, + * so any read past buf+len faults here instead of in a user's transfer. The + * buffers above deliberately carry 64 spare bytes for the unaligned case, which + * is exactly what let a 64-byte over-read in the AVX2 assembly go unnoticed: + * it only crashed when an allocation happened to end at a page boundary. */ +static int test_no_overread(void) +{ +#if defined HAVE_SYS_MMAN_H || defined __linux__ || defined __APPLE__ || defined BSD + /* Setting the guard up is not optional on the platforms this is compiled + * for: if it fails we are not testing what the caller thinks, so say so and + * fail rather than return a pass that looks identical to a real one. */ + long pagesz = sysconf(_SC_PAGESIZE); + if (pagesz <= 0) { + printf("FAIL guard-page: sysconf(_SC_PAGESIZE) gave %ld\n", pagesz); + return 1; + } + size_t region = (size_t)pagesz * 4; + char *base = (char *)mmap(NULL, region, PROT_READ | PROT_WRITE, + MAP_PRIVATE | MAP_ANONYMOUS, -1, 0); + if (base == MAP_FAILED) { + printf("FAIL guard-page: mmap failed\n"); + return 1; + } + if (mprotect(base + region - pagesz, pagesz, PROT_NONE) != 0) { + printf("FAIL guard-page: mprotect failed\n"); + munmap(base, region); + return 1; + } + /* Whether the ASSEMBLY was reached is a separate question from whether the + * checks passed. Without AVX2 the dispatcher falls back and the guard loop + * proves nothing about it, which must not read as coverage. */ + __builtin_cpu_init(); + printf("guard-page: AVX2 %s\n", + __builtin_cpu_supports("avx2") + ? "present, assembly and intrinsics exercised" + : "ABSENT -- fallback only, the AVX2 paths were NOT exercised"); + int failures = 0; + /* 128 is the shortest the AVX2 paths will touch; step by 1 so every + * remainder mod 64, and both alignments, get covered. */ + for (int32 len = 128; len <= 4096; len++) { + char *buf = base + region - pagesz - len; /* last byte abuts the guard */ + for (int32 i = 0; i < len; i++) + buf[i] = (char)((i + (i % 3) + (i % 11)) % 256); + uint32 ref = checksum_via_default(buf, len); + if (checksum_via_sse2(buf, len) != ref + || checksum_via_ssse3(buf, len) != ref + || checksum_via_avx2(buf, len) != ref + || get_checksum1(buf, len) != ref) { + printf("FAIL guard-page size=%5d: mismatch at the page boundary\n", len); + if (++failures > 4) + break; + } + } + munmap(base, region); + return failures; +#else + printf("guard-page: no mmap on this platform, over-read check NOT RUN\n"); + return 0; +#endif +} + int main() { static const int sizes[] = {1, 4, 31, 32, 33, 63, 64, 65, 128, 129, 256, 700, 1024, 4096, 65536}; @@ -649,6 +713,8 @@ int main() free(raw); + failures += test_no_overread(); + if (failures) { printf("%d checksum mismatches!\n", failures); return 1;