fix some documentation, comments, and tagging tests with new check

Author: struct

MEMORY_TAGGING.md

@@ -1,12 +1,12 @@
 # Isolation Alloc Memory Tagging
 
-IsoAlloc supports a software only memory tagging model that is conceptually very similar to Chromes [MTECheckedPtr](https://docs.google.com/document/d/1ph7iOorkGqTuETFZp-xvHV4L2rYootuz1ThzAAoGe30/edit?usp=sharing). This technique for pointer protection is inspired by ARM's upcoming Memory Tagging Extension (MTE) due in ARM v8.5-A. ARM MTE is a comprehensive hardware based solution for detecting memory safety issues in release builds of software with very little overhead. ARM MTE uses the Top Byte Ignore (TBI) feature to transparently tag pointers with metadata or a 'tag'. With ARM MTE this tag is mostly transparently checked and removed in hardware. The feature implemented here is conceptually very similar.
+IsoAlloc supports a software only memory tagging model that is very similar to Chromes [MTECheckedPtr](https://docs.google.com/document/d/1ph7iOorkGqTuETFZp-xvHV4L2rYootuz1ThzAAoGe30/edit?usp=sharing). This technique for pointer protection is inspired by ARM's upcoming Memory Tagging Extension (MTE) due in ARM v8.5-A. ARM MTE is a comprehensive hardware based solution for detecting memory safety issues in release builds of software with very little overhead. ARM MTE uses the Top Byte Ignore (TBI) feature to transparently tag pointers with metadata or a 'tag'. With ARM MTE this tag is mostly transparently checked and removed in hardware. The feature implemented here in IsoAlloc is conceptually very similar except that tagging and untagging of pointers happens in software.
 
 Note that this feature is experimental, off by default, and the APIs are subject to change!
 
 ## Overview
 
-We can't achieve the granularity provided by ARM MTE in software alone but we can implement a pointer protection mechanism by generating 1 byte of meta data per chunk managed by a private IsoAlloc zone, adding that tag to the pointer, and verifying it before derefencing it. This feature is enabled or disabled with `MEMORY_TAGGING` in the `Makefile`.
+We can't achieve the granularity provided by ARM MTE in software alone but we can implement a pointer protection mechanism by generating 1 byte of meta data per chunk managed by a private IsoAlloc zone, adding that tag to the pointer, and verifying it before dereferencing it. This feature is enabled or disabled with `MEMORY_TAGGING` in the `Makefile`.
 
 This 1 byte tag will be added to the LSB of the pointer returned by calling `iso_alloc_from_zone_tagged`. IsoAlloc also provides a C API for tagging and untagging pointers retrieved by `iso_alloc_from_zone`. These functions are `iso_alloc_tag_ptr` and `iso_alloc_untag_ptr`.
 
@@ -36,8 +36,8 @@ These APIs can also be used in C, but this requires tagging and untagging pointe
 
 * Currently only private zones can make use of memory tagging in IsoAlloc
 * All tag data is stored below user pages with a guard page allocated in between
-* A single 1 byte tag is generated per chunk in a private zone, this means the memory required to hold tags is larger for private zones holding smaller chunk sizes. For zones holding chunks 1024 byte or larger only a single of page of memory is required for tags as there are only 4096 possible 1024 byte chunks in zone user size of 4mb. The maximum amount of memory needed is for 16 byte chunks which requires 64 pages because there are 262144 possible chunks with a zone user size of 4mb.
-* Tags are 1 byte in size and randomly chosen, they are added to the LSB of a pointer (e.g. tag value `0xed`, tagged pointer `0xed0b8066c1a000`, untagged pointer `0xb8066c1a000`)
+* A single 1 byte tag is generated per chunk in a private zone, this means the memory required to hold tags is larger for private zones holding smaller chunk sizes. For zones holding chunks 1024 byte or larger only a single of page of memory is required for tags as there are only 4096 possible 1024 byte chunks in 4mb IsoAlloc zone. The maximum amount of memory needed is for 16 byte chunks which requires 64 pages because there are 262144 possible chunks with a 4mb zone.
+* Tags are 1 byte in size and randomly chosen, they are added to the LSB of a pointer (e.g. tag value `0xed`, tagged pointer `0xed000b8066c1a000`, untagged pointer `0xb8066c1a000`)
 * Tags are refreshed whenever the private zone has reached %25 of 'retirement age' (defined in conf.h as `ZONE_ALLOC_RETIRE`) with 0 current allocations
 
 ## Examples

Makefile

@@ -305,6 +305,7 @@ tests: clean library_debug_unit_tests
 	@echo "make tests"
 	$(CC) $(CFLAGS) $(EXE_CFLAGS) $(DEBUG_LOG_FLAGS) $(GDB_FLAGS) $(OS_FLAGS) tests/rand_freelist.c $(ISO_ALLOC_PRINTF_SRC) -o $(BUILD_DIR)/rand_freelist $(LDFLAGS)
 	$(CC) $(CFLAGS) $(EXE_CFLAGS) $(DEBUG_LOG_FLAGS) $(GDB_FLAGS) $(OS_FLAGS) tests/tagged_ptr_test.c $(ISO_ALLOC_PRINTF_SRC) -o $(BUILD_DIR)/tagged_ptr_test $(LDFLAGS)
+	$(CC) $(CFLAGS) $(EXE_CFLAGS) $(DEBUG_LOG_FLAGS) $(GDB_FLAGS) $(OS_FLAGS) tests/bad_tag_ptr_test.c $(ISO_ALLOC_PRINTF_SRC) -o $(BUILD_DIR)/bad_tag_ptr_test $(LDFLAGS)
 	$(CC) $(CFLAGS) $(EXE_CFLAGS) $(DEBUG_LOG_FLAGS) $(GDB_FLAGS) $(OS_FLAGS) tests/tests.c $(ISO_ALLOC_PRINTF_SRC) -o $(BUILD_DIR)/tests $(LDFLAGS)
 	$(CC) $(CFLAGS) $(EXE_CFLAGS) $(DEBUG_LOG_FLAGS) $(GDB_FLAGS) $(OS_FLAGS) tests/uaf.c $(ISO_ALLOC_PRINTF_SRC) -o $(BUILD_DIR)/uaf $(LDFLAGS)
 	$(CC) $(CFLAGS) $(EXE_CFLAGS) $(DEBUG_LOG_FLAGS) $(GDB_FLAGS) $(OS_FLAGS) tests/interfaces_test.c $(ISO_ALLOC_PRINTF_SRC) -o $(BUILD_DIR)/interfaces_test $(LDFLAGS)

include/iso_alloc_internal.h

@@ -76,6 +76,8 @@ assert(sizeof(size_t) >= 64)
 #define FREE_OR_DONTNEED MADV_FREE
 #endif
 
+static_assert(SMALLEST_CHUNK_SZ >= 16, "SMALLEST_CHUNK_SZ is too small, must be at least 16");
+
 #if ENABLE_ASAN
 #include <sanitizer/asan_interface.h>
 
@@ -239,6 +241,8 @@ assert(sizeof(size_t) >= 64)
 /* Cap our big zones at 4GB of memory */
 #define BIG_SZ_MAX 4294967296
 
+#define MIN_BITMAP_IDX 8
+
 #define WASTED_SZ_MULTIPLIER 8
 #define WASTED_SZ_MULTIPLIER_SHIFT 3
 

src/iso_alloc.c

@@ -490,7 +490,8 @@ INTERNAL_HIDDEN iso_alloc_zone_t *_iso_new_zone(size_t size, bool internal, int3
         create_guard_page(p + g_page_size + tag_mapping_size);
         new_zone->user_pages_start = (p + g_page_size + tag_mapping_size + g_page_size);
         uint64_t *_mtp = p + g_page_size;
-        int64_t tms = tag_mapping_size / sizeof(uint64_t);
+        /* (>> 3) == sizeof(uint64_t) == 8 */
+        uint64_t tms = tag_mapping_size >> 3;
 
         /* Generate random tags */
         for(uint64_t o = 0; o < tms; o++) {
@@ -587,9 +588,18 @@ INTERNAL_HIDDEN void fill_free_bit_slots(iso_alloc_zone_t *zone) {
      * leads to a less predictable free list */
     bitmap_index_t bm_idx = 0;
 
+    /* Refresh the random seed for wyrand. This only
+     * requires a single syscall to getrandom() */
+    _root->seed = rand_uint64();
+
     /* The largest zone->max_bitmap_idx we will ever
-     * have is 8192 for SMALLEST_CHUNK_SZ (16) */
-    if(zone->max_bitmap_idx > ALIGNMENT) {
+     * have is 8192 for SMALLEST_CHUNK_SZ (16). The
+     * smallest zone->max_bitmap_idx is 1 when chunk
+     * size is SMALL_SZ_MAX because the bitmap_size
+     * is only 8 bytes. If our max bitmap index is
+     * small then it won't provide enough search
+     * space for a random list to be of value */
+    if(zone->max_bitmap_idx > MIN_BITMAP_IDX) {
         bm_idx = ((uint32_t) us_rand_uint64(&_root->seed) & (zone->max_bitmap_idx - 1));
     }
 

src/iso_alloc_mem_tags.c

@@ -3,10 +3,16 @@
 
 #include "iso_alloc_internal.h"
 
+/* Returns a tag for a pointer p, which must be untagged
+ * when passed to this function */
 INTERNAL_HIDDEN uint8_t _iso_alloc_get_mem_tag(void *p, iso_alloc_zone_t *zone) {
 #if MEMORY_TAGGING
     void *user_pages_start = UNMASK_USER_PTR(zone);
 
+    if(user_pages_start > p || (user_pages_start + ZONE_USER_SIZE) < p) {
+        LOG_AND_ABORT("Cannot get tag for pointer %p with wrong zone %d %p - %p", p, zone->index, user_pages_start, user_pages_start + ZONE_USER_SIZE);
+    }
+
     uint8_t *_mtp = (user_pages_start - g_page_size - ROUND_UP_PAGE(zone->chunk_count * MEM_TAG_SIZE));
     const uint64_t chunk_offset = (uint64_t)(p - user_pages_start);
 

tests/bad_tag_ptr_test.c

@@ -0,0 +1,33 @@
+/* iso_alloc bad_tag_ptr_test.c
+ * Copyright 2022 - chris.rohlf@gmail.com */
+
+/* This test should successfully fail with or
+ * without MEMORY_TAGGING support */
+
+#include <stdio.h>
+#include <string.h>
+#include "iso_alloc.h"
+
+#define SIZE 256
+
+int main(int argc, char *argv[]) {
+    iso_alloc_zone_handle *_zone_handle = iso_alloc_new_zone(SIZE);
+
+    if(_zone_handle == NULL) {
+        abort();
+    }
+
+    char buffer[1024];
+    void *p = &buffer;
+
+    /* This should crash because p is not allocated from this zone */
+    uint8_t tag = iso_alloc_get_mem_tag(p, _zone_handle);
+    printf("Tag = %x\n", tag);
+    iso_alloc_destroy_zone(_zone_handle);
+
+#if !MEMORY_TAGGING
+    return -1;
+#endif
+
+    return 0;
+}

tests/tagged_ptr_test.c

@@ -1,4 +1,4 @@
-/* iso_alloc iso_alloc_tagged_ptr_test.c
+/* iso_alloc tagged_ptr_test.c
  * Copyright 2022 - chris.rohlf@gmail.com */
 
 /* This test should successfully run with or
@@ -7,6 +7,7 @@
 #include <stdio.h>
 #include <string.h>
 #include "iso_alloc.h"
+#include "iso_alloc_internal.h"
 
 #define SIZE 256
 
@@ -18,11 +19,14 @@ int main(int argc, char *argv[]) {
     }
 
     void *p = iso_alloc_from_zone_tagged(_zone_handle);
+    void *up = iso_alloc_untag_ptr(p, _zone_handle);
 
-    void *untagged_p = iso_alloc_untag_ptr(p, _zone_handle);
+    uint8_t tag = ((uintptr_t) p >> 56);
+    uint8_t itag = iso_alloc_get_mem_tag(up, _zone_handle);
 
-    /* This should crash if the pointer wasn't properly untagged */
-    memset(untagged_p, 0x41, SIZE);
+    if(tag != itag) {
+        LOG_AND_ABORT("Tags %d and %d do not match", tag, itag);
+    }
 
     /* We can pass a tagged or untagged pointer to iso_free_from_zone */
     iso_free_from_zone(p, _zone_handle);

tests/tagged_ptr_test.cpp

@@ -1,4 +1,4 @@
-// iso_alloc iso_alloc_tagged_ptr_test.cpp
+// iso_alloc tagged_ptr_test.cpp
 // Copyright 2022 - chris.rohlf@gmail.com
 
 // This test should successfully run with or

utils/run_tests.sh

@@ -31,7 +31,7 @@ done
 
 fail_tests=("double_free" "big_double_free" "heap_overflow" "heap_underflow"
             "leaks_test" "wild_free" "unaligned_free" "incorrect_chunk_size_multiple"
-            "big_canary_test" "zero_alloc" "sized_free")
+            "big_canary_test" "zero_alloc" "sized_free" "bad_tag_ptr_test")
 
 for t in "${fail_tests[@]}"; do
     echo -n "Running $t test"