<div dir="ltr"><div><div><div><div><div><div>I'm considering a commit containing some of the below additional checks:<br><br></div>- try to spot a double free (I think this check should go in)<br><br></div>- try to spot a use-after-free, not so sure about this<br><br></div>- and there is also the possibility of a check for heap overflow<br><br></div>but these start to stray into checks that free() is doing anyway :-/<br><br></div>thoughts,<br></div>Andrew<br><div><div><div><div><div><div><br>diff --git a/lib/libswan/alloc.c b/lib/libswan/alloc.c<br>index ab6f3194b..c1f9824e7 100644<br>--- a/lib/libswan/alloc.c<br>+++ b/lib/libswan/alloc.c<br>@@ -61,6 +61,20 @@ void set_alloc_exit_log_func(exit_log_func_t func)<br> /* this magic number is 3671129837 decimal (623837458 complemented) */<br> #define LEAK_MAGIC 0xDAD0FEEDul<br> <br>+#define INVALID_MAGIC 0xFE<br>+#if __SIZEOF_POINTER__ == 4<br>+#define INVALID_POINTER ((void*)0xFEFEFEFEul)<br>+#endif<br>+#if __SIZEOF_POINTER__ == 8<br>+#define INVALID_POINTER ((void*)0xFEFEFEFEFEFEFEFEull)<br>+#endif<br>+#ifndef INVALID_POINTER<br>+/* suppresses check */<br>+#define INVALID_POINTER NULL<br>+#endif<br>+<br>+static bool check_invalid_pointer = true;<br>+<br> union mhdr {<br>     struct {<br>         const char *name;<br>@@ -116,6 +130,9 @@ static void *alloc_bytes_raw(size_t size, const char *name)<br>             allocs = p;<br>             pthread_mutex_unlock(&leak_detective_mutex);<br>         }<br>+        if (p + 1 == INVALID_POINTER) {<br>+            check_invalid_pointer = false; /* sigh! */<br>+        }<br>         return p + 1;<br>     } else {<br>         return p;<br>@@ -128,8 +145,20 @@ void pfree(void *ptr)<br>         union mhdr *p;<br> <br>         passert(ptr != NULL);<br>+        if (check_invalid_pointer) {<br>+            if (ptr == INVALID_POINTER) {<br>+                PASSERT_FAIL("pointer %p invalid (likely from use after free)", p);<br>+            }<br>+        }<br>+<br>         p = ((union mhdr *)ptr) - 1;<br>-        passert(p->i.magic == LEAK_MAGIC);<br>+<br>+        if (p->i.magic == ~LEAK_MAGIC) {<br>+            PASSERT_FAIL("pointer %p invalid (likely from double free)", ptr);<br>+        } else if (p->i.magic != LEAK_MAGIC) {<br>+            PASSERT_FAIL("pointer %p invalid (likely from heap corruption)", ptr);<br>+        }<br>+<br>         {<br>             pthread_mutex_lock(&leak_detective_mutex);<br>             if (p->i.older != NULL) {<br>@@ -146,7 +175,7 @@ void pfree(void *ptr)<br>             pthread_mutex_unlock(&leak_detective_mutex);<br>         }<br>         /* stomp on memory!   Is another byte value better? */<br>-        memset(p, 0xEF, sizeof(union mhdr) + p->i.size);<br>+        memset(p, INVALID_MAGIC, sizeof(union mhdr) + p->i.size);<br>         p->i.magic = ~LEAK_MAGIC;<br>         free(p);<br>     } else {<br><br></div></div></div></div></div></div></div>