Merge pull request #117 from struct/memset_sanity

add memset_sanity and build cleanups to enable it

Author: struct

Makefile

@@ -103,10 +103,13 @@ SCHED_GETCPU =
 ## much of a performance penalty
 ALLOC_SANITY = -DALLOC_SANITY=0
 
-## Enable hooking of memcpy to detect out of bounds r/w
-## operations on chunks allocated with IsoAlloc. Does
-## not require ALLOC_SANITY is enabled
-MEMCPY_SANITY = -DMEMCPY_SANITY=0
+## Enable hooking of memcpy/memset to detect out of bounds
+## r/w operations on chunks allocated with IsoAlloc. Does
+## not require ALLOC_SANITY is enabled. On MacOS you need
+## to set FORTIFY_SOURCE to 0. Leave these commented if
+## you aren't enabling them.
+#MEMCPY_SANITY = -DMEMCPY_SANITY=0 -D_FORTIFY_SOURCE=0
+#MEMSET_SANITY = -DMEMSET_SANITY=0 -D_FORTIFY_SOURCE=0
 
 ## Enable the userfaultfd based uninitialized read detection
 ## feature. This samples calls to malloc, and allocates raw
@@ -219,7 +222,7 @@ CFLAGS = $(COMMON_CFLAGS) $(SECURITY_FLAGS) $(BUILD_ERROR_FLAGS) $(HOOKS) $(HEAP
 	-std=c11 $(SANITIZER_SUPPORT) $(ALLOC_SANITY) $(MEMCPY_SANITY) $(UNINIT_READ_SANITY) $(CPU_PIN) $(SCHED_GETCPU) \
 	$(EXPERIMENTAL) $(UAF_PTR_PAGE) $(VERIFY_BIT_SLOT_CACHE) $(NAMED_MAPPINGS) $(ABORT_ON_NULL) $(NO_ZERO_ALLOCATIONS) \
 	$(ABORT_NO_ENTROPY) $(ISO_DTOR_CLEANUP) $(SHUFFLE_BIT_SLOT_CACHE) $(USE_SPINLOCK) $(HUGE_PAGES) $(USE_MLOCK) \
-	$(MEMORY_TAGGING) $(STRONG_SIZE_ISOLATION)
+	$(MEMORY_TAGGING) $(STRONG_SIZE_ISOLATION) $(MEMSET_SANITY)
 CXXFLAGS = $(COMMON_CFLAGS) -DCPP_SUPPORT=1 -std=c++17 $(SANITIZER_SUPPORT) $(HOOKS)
 EXE_CFLAGS = -fPIE
 GDB_FLAGS = -g -ggdb3 -fno-omit-frame-pointer
@@ -304,9 +307,13 @@ tests: clean library_debug_unit_tests
 	$(CC) $(CFLAGS) $(EXE_CFLAGS) $(DEBUG_LOG_FLAGS) $(GDB_FLAGS) $(OS_FLAGS) tests/incorrect_chunk_size_multiple.c $(ISO_ALLOC_PRINTF_SRC) -o $(BUILD_DIR)/incorrect_chunk_size_multiple $(LDFLAGS)
 	$(CC) $(CFLAGS) $(EXE_CFLAGS) $(DEBUG_LOG_FLAGS) $(GDB_FLAGS) $(OS_FLAGS) tests/zero_alloc.c $(ISO_ALLOC_PRINTF_SRC) -o $(BUILD_DIR)/zero_alloc $(LDFLAGS)
 	$(CC) $(CFLAGS) $(EXE_CFLAGS) $(DEBUG_LOG_FLAGS) $(GDB_FLAGS) $(OS_FLAGS) tests/uninit_read.c $(ISO_ALLOC_PRINTF_SRC) -o $(BUILD_DIR)/uninit_read $(LDFLAGS)
-	$(CC) $(CFLAGS) $(EXE_CFLAGS) $(DEBUG_LOG_FLAGS) $(GDB_FLAGS) $(OS_FLAGS)  tests/sized_free.c $(ISO_ALLOC_PRINTF_SRC) -o $(BUILD_DIR)/sized_free $(LDFLAGS)
+	$(CC) $(CFLAGS) $(EXE_CFLAGS) $(DEBUG_LOG_FLAGS) $(GDB_FLAGS) $(OS_FLAGS) tests/sized_free.c $(ISO_ALLOC_PRINTF_SRC) -o $(BUILD_DIR)/sized_free $(LDFLAGS)
 	utils/run_tests.sh
 
+libc_sanity_tests: clean library_debug_unit_tests
+	$(CC) $(CFLAGS) $(EXE_CFLAGS) $(DEBUG_LOG_FLAGS) $(GDB_FLAGS) $(OS_FLAGS) tests/memset_sanity.c $(ISO_ALLOC_PRINTF_SRC) -o $(BUILD_DIR)/memset_sanity $(LDFLAGS)
+	$(CC) $(CFLAGS) $(EXE_CFLAGS) $(DEBUG_LOG_FLAGS) $(GDB_FLAGS) $(OS_FLAGS) tests/memcpy_sanity.c $(ISO_ALLOC_PRINTF_SRC) -o $(BUILD_DIR)/memcpy_sanity $(LDFLAGS)
+
 fuzz_test: clean library_debug_unit_tests
 	@echo "make fuzz_test"
 	$(CC) $(CFLAGS) $(EXE_CFLAGS) $(DEBUG_LOG_FLAGS) $(GDB_FLAGS) tests/alloc_fuzz.c $(ISO_ALLOC_PRINTF_SRC) -o $(BUILD_DIR)/alloc_fuzz $(LDFLAGS)

README.md

@@ -85,7 +85,7 @@ When enabled, the `CPU_PIN` feature will restrict allocations from a given zone
 * When destroying private zones if `NEVER_REUSE_ZONES` is enabled IsoAlloc won't attempt to repurpose the zone
 * Zones are retired and replaced after they've allocated and freed a specific number of chunks. This is calculated as `ZONE_ALLOC_RETIRE * max_chunk_count_for_zone`.
 * `MEMORY_TAGGING` When enabled IsoAlloc will create a 1 byte tag for each chunk in private zones. See the [MEMORY_TAGGING.md](MEMORY_TAGGING.md) documentation, or [this test](tests/tagged_ptr_test.cpp) for an example of how to use it.
-* `MEMCPY_SANITY` Configures the allocator will hook all calls to `memcpy` and check for out of bounds r/w operations when either src or dst points to a chunk allocated by IsoAlloc
+* `MEMCPY_SANITY` and `MEMSET_SANITY` Configures the allocator will hook all calls to `memcpy`/`memset` and check for out of bounds r/w operations when either src or dst points to a chunk allocated by IsoAlloc
 * `STRONG_SIZE_ISOLATION` Enables a policy that enforces stronger memory isolation by size
 
 ## Building
@@ -102,6 +102,8 @@ The Makefile targets are very simple:
 
 `make tests` - Builds and runs all tests
 
+`make libc_sanity_tests` - Builds the memcpy/memset libc hook sanity tests
+
 `make perf_tests` - Builds and runs a simple performance test that uses gprof. Linux only
 
 `make malloc_cmp_test` - Builds and runs a test that uses both iso_alloc and malloc for comparison

include/iso_alloc_sanity.h

@@ -86,3 +86,8 @@ INTERNAL_HIDDEN _sane_allocation_t *_get_sane_alloc(void *p);
 INTERNAL_HIDDEN void *__iso_memcpy(void *restrict dest, const void *restrict src, size_t n);
 INTERNAL_HIDDEN void *_iso_alloc_memcpy(void *restrict dest, const void *restrict src, size_t n);
 #endif
+
+#if MEMSET_SANITY
+INTERNAL_HIDDEN void *__iso_memset(void *dest, int b, size_t n);
+INTERNAL_HIDDEN void *_iso_alloc_memset(void *dest, int b, size_t n);
+#endif

src/iso_alloc_sanity.c

@@ -277,6 +277,9 @@ INTERNAL_HIDDEN void *_iso_alloc_sample(const size_t size) {
 #endif
 
 #if MEMCPY_SANITY
+
+#define MEMCPY_SANITY_CHK(p) (user_pages_start <= p && (user_pages_start + ZONE_USER_SIZE) - n > p && n > zone->chunk_size)
+
 INTERNAL_HIDDEN void *__iso_memcpy(void *restrict dest, const void *restrict src, size_t n) {
     char *p_dest = (char *) dest;
     char const *p_src = (char const *) src;
@@ -299,18 +302,46 @@ INTERNAL_HIDDEN void *_iso_alloc_memcpy(void *restrict dest, const void *restric
         iso_alloc_zone_t *zone = search_chunk_lookup_table(dest);
         void *user_pages_start = UNMASK_USER_PTR(zone);
 
-        if(user_pages_start <= dest && (user_pages_start + ZONE_USER_SIZE) > dest && n > zone->chunk_size) {
+        if(MEMCPY_SANITY_CHK(dest)) {
             LOG_AND_ABORT("Detected an out of bounds write memcpy: dest=0x%p (%d bytes) src=0x%p size=%d", dest, zone->chunk_size, src, n);
         }
 
         zone = search_chunk_lookup_table(src);
         user_pages_start = UNMASK_USER_PTR(zone);
 
-        if(user_pages_start <= src && (user_pages_start + ZONE_USER_SIZE) > src && n > zone->chunk_size) {
+        if(MEMCPY_SANITY_CHK(src)) {
             LOG_AND_ABORT("Detected an out of bounds read memcpy: dest=0x%p src=0x%p (%d bytes) size=%d", dest, src, zone->chunk_size, n);
         }
     }
 
     return __iso_memcpy(dest, src, n);
 }
 #endif
+
+#if MEMSET_SANITY
+
+#define MEMSET_SANITY_CHK(p) (user_pages_start <= p && (user_pages_start + ZONE_USER_SIZE) - n > p && n > zone->chunk_size)
+
+INTERNAL_HIDDEN void *__iso_memset(void *dest, int b, size_t n) {
+    char *p_dest = (char *) dest;
+
+    while(n--) {
+        *p_dest++ = b;
+    }
+
+    return dest;
+}
+
+INTERNAL_HIDDEN void *_iso_alloc_memset(void *dest, int b, size_t n) {
+    if(n > SMALLEST_CHUNK_SZ) {
+        iso_alloc_zone_t *zone = search_chunk_lookup_table(dest);
+        void *user_pages_start = UNMASK_USER_PTR(zone);
+
+        if(MEMSET_SANITY_CHK(dest)) {
+            LOG_AND_ABORT("Detected an out of bounds write memset: dest=0x%p (%d bytes) size=%d", dest, zone->chunk_size, n);
+        }
+    }
+
+    return __iso_memset(dest, b, n);
+}
+#endif

src/libc_hook.c

@@ -5,9 +5,13 @@
 #include "iso_alloc_sanity.h"
 
 #if MEMCPY_SANITY
-
 EXTERNAL_API void *memcpy(void *restrict dest, const void *restrict src, size_t n) {
     return _iso_alloc_memcpy(dest, src, n);
 }
+#endif
 
+#if MEMSET_SANITY
+EXTERNAL_API void *memset(void *dest, int b, size_t n) {
+    return _iso_alloc_memset(dest, b, n);
+}
 #endif

tests/heap_overflow.c

@@ -14,13 +14,16 @@ int main(int argc, char *argv[]) {
 
     p = (uint8_t *) iso_alloc(32);
 
-#if MEMCPY_SANITY
     const char *A = "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"
                     "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"
                     "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA";
+#if MEMCPY_SANITY
     memcpy(p, A, strlen(A));
 #else
-    memset(p, 0x41, 65535);
+    size_t n = strlen(A);
+    while(n--) {
+        *p++ = *A++;
+    }
 #endif
 
     iso_free(p);

tests/heap_underflow.c

@@ -13,7 +13,18 @@ int main(int argc, char *argv[]) {
     }
 
     p = (uint8_t *) iso_alloc(32);
+
+#if MEMSET_SANITY
+    uint8_t *p_dest = p - 65535;
+    size_t n = 65535;
+
+    while(n--) {
+        *p_dest++ = 0;
+    }
+#else
     memset(p - 65535, 0x42, 65535);
+#endif
+
     iso_free(p);
     iso_verify_zones();
 

tests/memcpy_sanity.c

@@ -0,0 +1,30 @@
+/* iso_alloc memcpy_sanity.c
+ * Copyright 2022 - chris.rohlf@gmail.com */
+
+#include "iso_alloc.h"
+#include "iso_alloc_internal.h"
+
+#if !MEMCPY_SANITY
+#error "This test intended to be run with -DMEMCPY_SANITY=1"
+#endif
+
+int main(int argc, char *argv[]) {
+    uint8_t *p = NULL;
+
+    for(int32_t i = 0; i < 1024; i++) {
+        p = (uint8_t *) iso_alloc(8);
+        iso_free(p);
+    }
+
+    p = (uint8_t *) iso_alloc(8);
+
+    const char *A = "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"
+                    "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA"
+                    "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA";
+    memcpy(p, A, strlen(A));
+
+    iso_free(p);
+    iso_verify_zones();
+
+    return OK;
+}

tests/memset_sanity.c

@@ -0,0 +1,26 @@
+/* iso_alloc memset_sanity.c
+ * Copyright 2022 - chris.rohlf@gmail.com */
+
+#include "iso_alloc.h"
+#include "iso_alloc_internal.h"
+
+#if !MEMSET_SANITY
+#error "This test intended to be run with -DMEMSET_SANITY=1"
+#endif
+
+int main(int argc, char *argv[]) {
+    uint8_t *p = NULL;
+
+    for(int32_t i = 0; i < 1024; i++) {
+        p = (uint8_t *) iso_alloc(32);
+        iso_free(p);
+    }
+
+    p = (uint8_t *) iso_alloc(32);
+    memset(p, 0x41, 65535);
+
+    iso_free(p);
+    iso_verify_zones();
+
+    return OK;
+}