From 77134e1ad072aa3bf3fd5e225f58ae88b48db589 Mon Sep 17 00:00:00 2001
From: daan <daanl@outlook.com>
Date: Sat, 28 Dec 2019 15:17:49 -0800
Subject: [PATCH] update free list encoding to stronger formula with addition
 last

---
 include/mimalloc-internal.h | 29 +++++++++++++++++------------
 src/page.c                  |  2 +-
 2 files changed, 18 insertions(+), 13 deletions(-)

diff --git a/include/mimalloc-internal.h b/include/mimalloc-internal.h
index cdaac963..d41dfadc 100644
--- a/include/mimalloc-internal.h
+++ b/include/mimalloc-internal.h
@@ -397,24 +397,26 @@ Encoding/Decoding the free list next pointers
 
 This is to protect against buffer overflow exploits where the 
 free list is mutated. Many hardened allocators xor the next pointer `p` 
-with a secret key `k1`, as `p^k1`, but if the attacker can guess 
+with a secret key `k1`, as `p^k1`. This prevents overwriting with known
+values but might be still too weak: if the attacker can guess 
 the pointer `p` this  can reveal `k1` (since `p^k1^p == k1`). 
-Moreover, if multiple blocks can be read, the attacker can
+Moreover, if multiple blocks can be read as well, the attacker can
 xor both as `(p1^k1) ^ (p2^k1) == p1^p2` which may reveal a lot
 about the pointers (and subsequently `k1`).
 
-Instead mimalloc uses an extra key `k2` and encode as `rotl(p+k2,13)^k1`.
+Instead mimalloc uses an extra key `k2` and encodes as `((p^k2)<<<k1)+k1`.
 Since these operations are not associative, the above approaches do not
-work so well any more even if the `p` can be guesstimated. (We include 
-the rotation since xor and addition are otherwise linear in the lowest bit)
-Both keys are unique per page.
+work so well any more even if the `p` can be guesstimated. For example,
+for the read case we can subtract two entries to discard the `+k1` term, 
+but that leads to `((p1^k2)<<<k1) - ((p2^k2)<<<k1)` at best.
+We include the left-rotation since xor and addition are otherwise linear 
+in the lowest bit. Finally, both keys are unique per page which reduces
+the re-use of keys by a large factor.
 
 We also pass a separate `null` value to be used as `NULL` or otherwise
-`rotl(k2,13)^k1` would appear (too) often as a sentinel value.
+`(k2<<<k1)+k1` would appear (too) often as a sentinel value.
 ------------------------------------------------------------------- */
 
-#define MI_ENCODE_ROTATE_BITS (13)
-
 static inline bool mi_is_in_same_segment(const void* p, const void* q) {
   return (_mi_ptr_segment(p) == _mi_ptr_segment(q));
 }
@@ -429,14 +431,17 @@ static inline bool mi_is_in_same_page(const void* p, const void* q) {
 }
 
 static inline uintptr_t mi_rotl(uintptr_t x, uintptr_t shift) {
+  shift %= MI_INTPTR_BITS;
   return ((x << shift) | (x >> (MI_INTPTR_BITS - shift)));
 }
 static inline uintptr_t mi_rotr(uintptr_t x, uintptr_t shift) {
+  shift %= MI_INTPTR_BITS;
   return ((x >> shift) | (x << (MI_INTPTR_BITS - shift)));
 }
+
 static inline mi_block_t* mi_block_nextx( const void* null, const mi_block_t* block, uintptr_t key1, uintptr_t key2 ) {
   #ifdef MI_ENCODE_FREELIST
-  mi_block_t* b = (mi_block_t*)(mi_rotr(block->next ^ key1, MI_ENCODE_ROTATE_BITS) - key2);
+  mi_block_t* b = (mi_block_t*)(mi_rotr(block->next - key1, key1) ^ key2);
   if (mi_unlikely((void*)b==null)) { b = NULL; }
   return b;
   #else
@@ -448,7 +453,7 @@ static inline mi_block_t* mi_block_nextx( const void* null, const mi_block_t* bl
 static inline void mi_block_set_nextx(const void* null, mi_block_t* block, const mi_block_t* next, uintptr_t key1, uintptr_t key2) {
   #ifdef MI_ENCODE_FREELIST
   if (mi_unlikely(next==NULL)) { next = (mi_block_t*)null; }
-  block->next = mi_rotl((mi_encoded_t)next + key2, MI_ENCODE_ROTATE_BITS) ^ key1;
+  block->next = mi_rotl((uintptr_t)next ^ key2, key1) + key1;
   #else
   UNUSED(key1); UNUSED(key2); UNUSED(null);
   block->next = (mi_encoded_t)next;
@@ -485,7 +490,7 @@ static inline void mi_block_set_next(const mi_page_t* page, mi_block_t* block, c
 // -------------------------------------------------------------------
 
 static inline uintptr_t _mi_random_shuffle(uintptr_t x) {
-  mi_assert_internal(x!=0);
+  if (x==0) { x = 17; }   // ensure we don't get stuck in generating zeros
 #if (MI_INTPTR_SIZE==8)
   // by Sebastiano Vigna, see: <http://xoshiro.di.unimi.it/splitmix64.c>
   x ^= x >> 30;
diff --git a/src/page.c b/src/page.c
index 901fbda1..b070e56a 100644
--- a/src/page.c
+++ b/src/page.c
@@ -479,7 +479,7 @@ static void mi_page_free_list_extend_secure(mi_heap_t* const heap, mi_page_t* co
   counts[current]--;
   mi_block_t* const free_start = blocks[current];
   // and iterate through the rest; use `random_shuffle` for performance
-  uintptr_t rnd = _mi_random_shuffle(r);
+  uintptr_t rnd = _mi_random_shuffle(r|1); // ensure not 0
   for (size_t i = 1; i < extend; i++) {
     // call random_shuffle only every INTPTR_SIZE rounds
     const size_t round = i%MI_INTPTR_SIZE;