[libmicrohttpd] 05/05: Refactored user-poison: minimized scope of non-sanitized code

[gnunet]

This is an automated email from the git hooks/post-receive script.

karlson2k pushed a commit to branch master
in repository libmicrohttpd.

commit 3444792f14d96a58648add92b8f3b70e4babf772
Author: Evgeny Grin (Karlson2k) <k2k@narod.ru>
AuthorDate: Thu Nov 24 11:59:38 2022 +0300

    Refactored user-poison: minimized scope of non-sanitized code
---
 configure.ac                | 246 +++++++++++++++++++++++++++++++++++++-------
 src/include/mhd_options.h   |   7 +-
 src/microhttpd/memorypool.c | 120 ++++++++++++++++-----
 3 files changed, 308 insertions(+), 65 deletions(-)

diff --git a/configure.ac b/configure.ac
index cf261ead..b032badc 100644
--- a/configure.ac
+++ b/configure.ac
@@ -3995,39 +3995,167 @@ AS_VAR_IF([enable_sanitizers], ["no"], [:],
                AS_VAR_IF([enable_san_upoison], ["no"], [:],
                  [
                    AC_CHECK_HEADERS([sanitizer/asan_interface.h], [], [], 
[AC_INCLUDES_DEFAULT])
-                   AS_IF([test "x${mhd_cv_cc_sanitizer_pointer_compare}" = 
"xyes" && test "x${ac_cv_header_sanitizer_asan_interface_h}" = "xyes"],
+                   AS_VAR_IF([ac_cv_header_sanitizer_asan_interface_h],["yes"],
                      [
-                       AC_CACHE_CHECK([whether 
'__attribute__((no_sanitize("pointer-compare","pointer-subtract")))' works], 
[mhd_cv_func_attribute_nosanitize_ptr],
+                       AC_CACHE_CHECK([whether special function attribute is 
needed for user-poison], [mhd_cv_func_u_p_attribute_needed],
                          [
                            
ASAN_OPTIONS="exitcode=88:detect_invalid_pointer_pairs=3:halt_on_error=1"
                            export ASAN_OPTIONS
                            CFLAGS="${CFLAGS_ac} ${san_CFLAGS} ${san_FLAGS} 
${errattr_CFLAGS} ${user_CFLAGS}"
                            AC_RUN_IFELSE(
                              [
-                               AC_LANG_PROGRAM(
+                               AC_LANG_SOURCE(
                                  [[
+#include <stdint.h>
 #include <stdlib.h>
+#include <sanitizer/asan_interface.h>
 
-__attribute__((no_sanitize("pointer-compare","pointer-subtract")))
-int ptr_process(void *ptr1, void *ptr2)
+static const size_t first_pos = 0;
+static const size_t mid_pos = 64;
+static const size_t last_pos = 128;
+static const size_t zone_size = 16;
+static const size_t buf_size = 128 + 16;
+
+static int ptr_compare(void *ptr1, uint8_t *ptr2)
 {
-  if ((char*)ptr1 <= (char*)ptr2)
-    return (int) ((char*)ptr2 - (char*)ptr1);
-  return (int) ((char*)ptr1 - (char*)ptr2);
+  if ((((uintptr_t) (uint8_t *)ptr1) >= ((uintptr_t)ptr2)))
+    return ((char *) ptr1)[0] < ((char *) ptr2)[0];
+  return ((char *) ptr1)[0] > ((char *) ptr2)[0];
 }
-                                 ]],
+
+static int ptr_subtract(void *ptr1, uint8_t *ptr2)
+{
+  return ((size_t)(((uintptr_t)(uint8_t*)ptr1) - ((uintptr_t)ptr2))) <= 
last_pos;
+}
+
+int main(int argc, char *argv[])
+{
+  char *buf = (char*) malloc (buf_size);
+  char *a;
+  char *b;
+  int ret;
+
+  (void) argv;
+  if (NULL == buf)
+    return 10;
+  ASAN_POISON_MEMORY_REGION (buf + first_pos + zone_size, mid_pos - first_pos 
- zone_size);
+  ASAN_POISON_MEMORY_REGION (buf + mid_pos + zone_size, last_pos - mid_pos - 
zone_size);
+
+  if (0 < argc)
+    a = buf + last_pos;
+  else
+    a = buf + first_pos;
+  b = buf + mid_pos;
+
+  *a = '0';
+  *b = '9';
+
+  if (ptr_compare((void *)a, (uint8_t*) b))
+  {
+    if (ptr_subtract((void *)a, (uint8_t*) b))
+      ret = 0;
+    else
+      ret = 10;
+  }
+  else
+    ret = 5;
+  ASAN_UNPOISON_MEMORY_REGION (buf, buf_size);
+  free (buf);
+
+  return ret;
+}
+                                 ]]
+                               )
+                             ],
+                             [mhd_cv_func_u_p_attribute_needed="no"], 
[mhd_cv_func_u_p_attribute_needed="yes"],
+                             [
+                               # Cross-compiling with sanitizers??
+                               mhd_cv_func_up_attribute_needed='assuming no'
+                             ]
+                           )
+                           AS_UNSET([ASAN_OPTIONS])
+                         ]
+                       )
+                     ]
+                   )
+                   AS_VAR_IF([mhd_cv_func_u_p_attribute_needed],["yes"],[:],
+                     [
+                       
AC_DEFINE([FUNC_PTRCOMPARE_CAST_WORKAROUND_WORKS],[1],[Define to '1' if cast to 
'uintptr_t' works for safely processing user-poisoned pointer])
+                     ]
+                   )
+                   AS_IF([test "x${mhd_cv_func_u_p_attribute_needed}" = "xyes" 
&& test "x${ac_cv_header_sanitizer_asan_interface_h}" = "xyes"],
+                     [
+                       AC_CACHE_CHECK([whether 
'__attribute__((no_sanitize("pointer-compare")))' and 
'__attribute__((no_sanitize("pointer-subtract")))' work],
+                         [mhd_cv_func_attribute_nosanitize_ptr],
+                         [
+                           
ASAN_OPTIONS="exitcode=88:detect_invalid_pointer_pairs=3:halt_on_error=1"
+                           export ASAN_OPTIONS
+                           CFLAGS="${CFLAGS_ac} ${san_CFLAGS} ${san_FLAGS} 
${errattr_CFLAGS} ${user_CFLAGS}"
+                           AC_RUN_IFELSE(
+                             [
+                               AC_LANG_SOURCE(
                                  [[
-  int *a = (int*) malloc (sizeof(int)*4);
-  int *b = (int*) malloc (sizeof(long)*6);
-  int c = ptr_process(a, b);
-  if (c)
+#include <stdint.h>
+#include <stdlib.h>
+#include <sanitizer/asan_interface.h>
+
+static const size_t first_pos = 0;
+static const size_t mid_pos = 64;
+static const size_t last_pos = 128;
+static const size_t zone_size = 16;
+static const size_t buf_size = 128 + 16;
+
+__attribute__((no_sanitize("pointer-compare")))
+static int ptr_compare(void *ptr1, uint8_t *ptr2)
+{
+  if ((((const uint8_t*)ptr1) >= ((const uint8_t*)ptr2)))
+    return ((char *) ptr1)[0] < ((char *) ptr2)[0];
+  return ((char *) ptr1)[0] > ((char *) ptr2)[0];
+}
+
+__attribute__((no_sanitize("pointer-subtract")))
+static int ptr_subtract(void *ptr1, uint8_t *ptr2)
+{
+  return ((size_t)(((const uint8_t*)ptr1) - \
+          ((const uint8_t*)ptr2))) <= last_pos;
+}
+
+int main(int argc, char *argv[])
+{
+  char *buf = (char*) malloc (buf_size);
+  char *a;
+  char *b;
+  int ret;
+
+  (void) argv;
+  if (NULL == buf)
+    return 10;
+  ASAN_POISON_MEMORY_REGION (buf + first_pos + zone_size, mid_pos - first_pos 
- zone_size);
+  ASAN_POISON_MEMORY_REGION (buf + mid_pos + zone_size, last_pos - mid_pos - 
zone_size);
+
+  if (0 < argc)
+    a = buf + last_pos;
+  else
+    a = buf + first_pos;
+  b = buf + mid_pos;
+
+  *a = '0';
+  *b = '9';
+
+  if (ptr_compare((void *)a, (uint8_t*) b))
   {
-    free (b);
-    free (a);
-    return 0;
+    if (ptr_subtract((void *)a, (uint8_t*) b))
+      ret = 0;
+    else
+      ret = 10;
   }
-  free (a);
-  free (b);
+  else
+    ret = 5;
+  ASAN_UNPOISON_MEMORY_REGION (buf, buf_size);
+  free (buf);
+
+  return ret;
+}
                                  ]]
                                )
                              ],
@@ -4041,7 +4169,10 @@ int ptr_process(void *ptr1, void *ptr2)
                          ]
                        )
                        AS_VAR_IF([mhd_cv_func_attribute_nosanitize_ptr], 
["yes"],
-                         [AC_DEFINE([FUNC_ATTR_PTRCOMPARE_WORKS],[1],[Define 
to '1' if '__attribute__((no_sanitize("pointer-compare","pointer-subtract")))' 
works])],
+                         [
+                           AC_DEFINE([FUNC_ATTR_PTRCOMPARE_WORKS],[1],[Define 
to '1' if '__attribute__((no_sanitize("pointer-compare")))' works])
+                           AC_DEFINE([FUNC_ATTR_PTRSUBTRACT_WORKS],[1],[Define 
to '1' if '__attribute__((no_sanitize("pointer-subtract")))' works])
+                         ],
                          [
                            AC_CACHE_CHECK([whether 
'__attribute__((no_sanitize("address")))' works for pointers compare], 
[mhd_cv_func_attribute_nosanitize_addr],
                              [
@@ -4050,30 +4181,69 @@ int ptr_process(void *ptr1, void *ptr2)
                                CFLAGS="${CFLAGS_ac} ${san_CFLAGS} ${san_FLAGS} 
${errattr_CFLAGS} ${user_CFLAGS}"
                                AC_RUN_IFELSE(
                                  [
-                                   AC_LANG_PROGRAM(
+                                   AC_LANG_SOURCE(
                                      [[
+#include <stdint.h>
 #include <stdlib.h>
+#include <sanitizer/asan_interface.h>
+
+static const size_t first_pos = 0;
+static const size_t mid_pos = 64;
+static const size_t last_pos = 128;
+static const size_t zone_size = 16;
+static const size_t buf_size = 128 + 16;
 
 __attribute__((no_sanitize("address")))
-int ptr_process(void *ptr1, void *ptr2)
+static int ptr_compare(void *ptr1, uint8_t *ptr2)
 {
-  if ((char*)ptr1 <= (char*)ptr2)
-    return (int) ((char*)ptr2 - (char*)ptr1);
-  return (int) ((char*)ptr1 - (char*)ptr2);
+  if ((((const uint8_t*)ptr1) >= ((const uint8_t*)ptr2)))
+    return ((char *) ptr1)[0] < ((char *) ptr2)[0];
+  return ((char *) ptr1)[0] > ((char *) ptr2)[0];
 }
-                                     ]],
-                                     [[
-  int *a = (int*) malloc (sizeof(int)*4);
-  int *b = (int*) malloc (sizeof(long)*6);
-  int c = ptr_process(a, b);
-  if (c)
+
+__attribute__((no_sanitize("address")))
+static int ptr_subtract(void *ptr1, uint8_t *ptr2)
+{
+  return ((size_t)(((const uint8_t*)ptr1) - \
+          ((const uint8_t*)ptr2))) <= last_pos;
+}
+
+int main(int argc, char *argv[])
+{
+  char *buf = (char*) malloc (buf_size);
+  char *a;
+  char *b;
+  int ret;
+
+  (void) argv;
+  if (NULL == buf)
+    return 10;
+  ASAN_POISON_MEMORY_REGION (buf + first_pos + zone_size, mid_pos - first_pos 
- zone_size);
+  ASAN_POISON_MEMORY_REGION (buf + mid_pos + zone_size, last_pos - mid_pos - 
zone_size);
+
+  if (0 < argc)
+    a = buf + last_pos;
+  else
+    a = buf + first_pos;
+  b = buf + mid_pos;
+
+  *a = '0';
+  *b = '9';
+
+  if (ptr_compare((void *)a, (uint8_t*) b))
   {
-    free (b);
-    free (a);
-    return 0;
+    if (ptr_subtract((void *)a, (uint8_t*) b))
+      ret = 0;
+    else
+      ret = 10;
   }
-  free (a);
-  free (b);
+  else
+    ret = 5;
+  ASAN_UNPOISON_MEMORY_REGION (buf, buf_size);
+  free (buf);
+
+  return ret;
+}
                                      ]]
                                    )
                                  ],
@@ -4263,9 +4433,9 @@ int main(void)
            AS_VAR_IF([enable_san_upoison], ["no"], [:],
              [
                AC_MSG_CHECKING([whether to enable user memory poisoning])
-               AS_IF([test "x${mhd_cv_cc_sanitizer_address}" = "xyes" && test 
"x${mhd_cv_cc_sanitizer_pointer_compare}" = "xyes" && \
-                 test "x${ac_cv_header_sanitizer_asan_interface_h}" = "xyes" 
&& \
-                 (test "x${mhd_cv_func_attribute_nosanitize_ptr}" = "xyes" || 
test "x${mhd_cv_func_attribute_nosanitize_addr}" = "xyes")],
+               AS_IF([test "x${mhd_cv_cc_sanitizer_address}" = "xyes" && test 
"x${ac_cv_header_sanitizer_asan_interface_h}" = "xyes" && \
+                 (test "x${mhd_cv_func_u_p_attribute_needed}" != "xyes" || 
test "x${mhd_cv_func_attribute_nosanitize_ptr}" = "xyes" || \
+                  test "x${mhd_cv_func_attribute_nosanitize_addr}" = "xyes")],
                  [
                    AC_DEFINE([MHD_ASAN_POISON_ACTIVE], [1], [Define to '1' if 
user memory poison is used])
                    
enabled_sanitizers="${enabled_sanitizers}${enabled_sanitizers:+, }user-poison"
diff --git a/src/include/mhd_options.h b/src/include/mhd_options.h
index 85ad6555..5a5b7fee 100644
--- a/src/include/mhd_options.h
+++ b/src/include/mhd_options.h
@@ -156,9 +156,12 @@
 #endif /* MHD_ASAN_ACTIVE */
 
 #if defined(MHD_ASAN_ACTIVE) && defined(HAVE_SANITIZER_ASAN_INTERFACE_H) && \
-  (defined(FUNC_ATTR_PTRCOMPARE_WORKS) || defined(FUNC_ATTR_NOSANITIZE_WORKS))
+  (defined(FUNC_PTRCOMPARE_CAST_WORKAROUND_WORKS) || \
+  (defined(FUNC_ATTR_PTRCOMPARE_WORKS) && \
+  defined(FUNC_ATTR_PTRSUBTRACT_WORKS)) || \
+  defined(FUNC_ATTR_NOSANITIZE_WORKS))
 #ifndef MHD_ASAN_POISON_ACTIVE
-/* Manual ASAN poisoning could be used */
+/* User ASAN poisoning could be used */
 #warning User memory poisoning is not active
 #endif /* ! MHD_ASAN_POISON_ACTIVE */
 #else  /* ! (MHD_ASAN_ACTIVE && HAVE_SANITIZER_ASAN_INTERFACE_H &&
diff --git a/src/microhttpd/memorypool.c b/src/microhttpd/memorypool.c
index 5d656b47..92ec6edc 100644
--- a/src/microhttpd/memorypool.c
+++ b/src/microhttpd/memorypool.c
@@ -102,19 +102,92 @@
 #define ROUND_TO_ALIGN_PLUS_RED_ZONE(n) ROUND_TO_ALIGN(n)
 #define _MHD_POISON_MEMORY(pointer, size) (void)0
 #define _MHD_UNPOISON_MEMORY(pointer, size) (void)0
+/**
+ * Boolean 'true' if the first pointer is less or equal the second pointer
+ */
+#define mp_ptr_le_(p1,p2) \
+  (((const uint8_t*)p1) <= ((const uint8_t*)p2))
+/**
+ * The difference in bytes between positions of the first and
+ * the second pointers
+ */
+#define mp_ptr_diff_(p1,p2) \
+  ((size_t)(((const uint8_t*)p1) - ((const uint8_t*)p2)))
 #else  /* MHD_ASAN_POISON_ACTIVE */
-#if defined(FUNC_ATTR_PTRCOMPARE_WORKS)
-#define _MHD_NOSANITIZE_PTRS \
-  __attribute__((no_sanitize("pointer-compare","pointer-subtract")))
-#elif defined(FUNC_ATTR_NOSANITIZE_WORKS)
-#define _MHD_NOSANITIZE_PTRS __attribute__((no_sanitize("address")))
-#endif
 #define _MHD_RED_ZONE_SIZE (ALIGN_SIZE)
 #define ROUND_TO_ALIGN_PLUS_RED_ZONE(n) (ROUND_TO_ALIGN(n) + 
_MHD_RED_ZONE_SIZE)
 #define _MHD_POISON_MEMORY(pointer, size) \
   ASAN_POISON_MEMORY_REGION ((pointer), (size))
 #define _MHD_UNPOISON_MEMORY(pointer, size) \
   ASAN_UNPOISON_MEMORY_REGION ((pointer), (size))
+#if defined(FUNC_PTRCOMPARE_CAST_WORKAROUND_WORKS)
+/**
+ * Boolean 'true' if the first pointer is less or equal the second pointer
+ */
+#define mp_ptr_le_(p1,p2) \
+  (((uintptr_t)((const void*)(p1))) <= ((uintptr_t)((const void*)(p1))))
+/**
+ * The difference in bytes between positions of the first and
+ * the second pointers
+ */
+#define mp_ptr_diff_(p1,p2) \
+  ((size_t)(((uintptr_t)((const uint8_t*)p1)) - \
+            ((uintptr_t)((const uint8_t*)p2))))
+#elif defined(FUNC_ATTR_PTRCOMPARE_WORKS) && \
+  defined(FUNC_ATTR_PTRSUBTRACT_WORKS)
+#ifdef _DEBUG
+/**
+ * Boolean 'true' if the first pointer is less or equal the second pointer
+ */
+__attribute__((no_sanitize ("pointer-compare"))) static bool
+mp_ptr_le_ (const void *p1, const void *p2)
+{
+  return (((const uint8_t *) p1) <= ((const uint8_t *) p2));
+}
+
+
+#endif /* _DEBUG */
+
+
+/**
+ * The difference in bytes between positions of the first and
+ * the second pointers
+ */
+__attribute__((no_sanitize ("pointer-subtract"))) static size_t
+mp_ptr_diff_ (const void *p1, const void *p2)
+{
+  return (size_t) (((const uint8_t *) p1) - ((const uint8_t *) p2));
+}
+
+
+#elif defined(FUNC_ATTR_NOSANITIZE_WORKS)
+#ifdef _DEBUG
+/**
+ * Boolean 'true' if the first pointer is less or equal the second pointer
+ */
+__attribute__((no_sanitize ("address"))) static bool
+mp_ptr_le_ (const void *p1, const void *p2)
+{
+  return (((const uint8_t *) p1) <= ((const uint8_t *) p2));
+}
+
+
+#endif /* _DEBUG */
+
+/**
+ * The difference in bytes between positions of the first and
+ * the second pointers
+ */
+__attribute__((no_sanitize ("address"))) static size_t
+mp_ptr_diff_ (const void *p1, const void *p2)
+{
+  return (size_t) (((const uint8_t *) p1) - ((const uint8_t *) p2));
+}
+
+
+#else  /* ! FUNC_ATTR_NOSANITIZE_WORKS */
+#error User-poisoning cannot be used
+#endif /* ! FUNC_ATTR_NOSANITIZE_WORKS */
 #endif /* MHD_ASAN_POISON_ACTIVE */
 
 /**
@@ -419,7 +492,7 @@ MHD_pool_try_alloc (struct MemoryPool *pool,
  *         NULL if the pool cannot support @a new_size
  *         bytes (old continues to be valid for @a old_size)
  */
-_MHD_NOSANITIZE_PTRS void *
+void *
 MHD_pool_reallocate (struct MemoryPool *pool,
                      void *old,
                      size_t old_size,
@@ -432,23 +505,21 @@ MHD_pool_reallocate (struct MemoryPool *pool,
   mhd_assert (pool->size >= pool->end - pool->pos);
   mhd_assert (old != NULL || old_size == 0);
   mhd_assert (pool->size >= old_size);
-  mhd_assert (old == NULL || pool->memory <= (uint8_t *) old);
-  /* (old == NULL || pool->memory + pool->size >= (uint8_t*) old + old_size) */
-  mhd_assert (old == NULL || \
-              (pool->size - _MHD_RED_ZONE_SIZE) >= \
-              (((size_t) (((uint8_t *) old) - pool->memory)) + old_size));
-  /* Blocks "from the end" must not be reallocated */
-  /* (old == NULL || old_size == 0 || pool->memory + pool->pos > (uint8_t*) 
old) */
-  mhd_assert (old == NULL || old_size == 0 || \
-              pool->pos > (size_t) ((uint8_t *) old - pool->memory));
-  mhd_assert (old == NULL || old_size == 0 || \
-              (size_t) (((uint8_t *) old) - pool->memory) + old_size <= \
-              pool->end - _MHD_RED_ZONE_SIZE);
 
   if (NULL != old)
   {   /* Have previously allocated data */
-    const size_t old_offset = (size_t) (((uint8_t *) old) - pool->memory);
+    const size_t old_offset = mp_ptr_diff_ (old, pool->memory);
     const bool shrinking = (old_size > new_size);
+
+    mhd_assert (mp_ptr_le_ (pool->memory, old));
+    /* (pool->memory + pool->size >= (uint8_t*) old + old_size) */
+    mhd_assert ((pool->size - _MHD_RED_ZONE_SIZE) >= (old_offset + old_size));
+    /* Blocks "from the end" must not be reallocated */
+    /* (old_size == 0 || pool->memory + pool->pos > (uint8_t*) old) */
+    mhd_assert ((old_size == 0) || \
+                (pool->pos > old_offset));
+    mhd_assert ((old_size == 0) || \
+                ((pool->end - _MHD_RED_ZONE_SIZE) >= (old_offset + old_size)));
     /* Try resizing in-place */
     if (shrinking)
     {     /* Shrinking in-place, zero-out freed part */
@@ -510,7 +581,7 @@ MHD_pool_reallocate (struct MemoryPool *pool,
  *                 (should be larger or equal to @a copy_bytes)
  * @return addr new address of @a keep (if it had to change)
  */
-_MHD_NOSANITIZE_PTRS void *
+void *
 MHD_pool_reset (struct MemoryPool *pool,
                 void *keep,
                 size_t copy_bytes,
@@ -521,11 +592,10 @@ MHD_pool_reset (struct MemoryPool *pool,
   mhd_assert (copy_bytes <= new_size);
   mhd_assert (copy_bytes <= pool->size);
   mhd_assert (keep != NULL || copy_bytes == 0);
-  mhd_assert (keep == NULL || pool->memory <= (uint8_t *) keep);
+  mhd_assert (keep == NULL || mp_ptr_le_ (pool->memory, keep));
   /* (keep == NULL || pool->memory + pool->size >= (uint8_t*) keep + 
copy_bytes) */
-  mhd_assert (keep == NULL || \
-              pool->size >= \
-              ((size_t) ((uint8_t *) keep - pool->memory)) + copy_bytes);
+  mhd_assert ((keep == NULL) || \
+              (pool->size >= mp_ptr_diff_ (keep, pool->memory) + copy_bytes));
   _MHD_UNPOISON_MEMORY (pool->memory, new_size);
   if ( (NULL != keep) &&
        (keep != pool->memory) )

-- 
To stop receiving notification emails like this one, please contact
gnunet@gnunet.org.