[Pixman] [PATCH 1/5] Add fence_malloc() and fence_free().

[Soeren Sandmann]

Søren Sandmann <sandmann at daimi.au.dk> writes:

> From: Søren Sandmann Pedersen <ssp at redhat.com>
> 
> These variants of malloc() and free() try to surround the allocated
> memory with protected pages so that out-of-bounds accessess will cause
> a segmentation fault.
> 
> If mprotect() and getpagesize() are not available, these functions are
> simply equivalent to malloc() and free().

This patch was pretty broken, in two ways. 

> +AC_CHECK_FUNC(getpagesize, have_getpagesize=yes, have_=no)
                                                         ^ 
A typo here.

> --- a/test/utils.c
> +++ b/test/utils.c
> @@ -5,6 +5,8 @@
>  #include <unistd.h>
>  #endif
>  
> +#include <sys/mman.h>
> +

This should be guarded by HAVE_SYS_MMAN_H.

New version below.

Soren


commit 9b735ad08f9ad1b150534739a9d7ac66ae67d9de
Author: Søren Sandmann Pedersen <ssp at redhat.com>
Date:   Mon Sep 13 14:34:34 2010 -0400

    Add fence_malloc() and fence_free().
    
    These variants of malloc() and free() try to surround the allocated
    memory with protected pages so that out-of-bounds accessess will cause
    a segmentation fault.
    
    If mprotect() and getpagesize() are not available, these functions are
    simply equivalent to malloc() and free().

diff --git a/configure.ac b/configure.ac
index dbff2a6..d3b71fa 100644
--- a/configure.ac
+++ b/configure.ac
@@ -623,6 +623,19 @@ if test x$have_alarm = xyes; then
    AC_DEFINE(HAVE_ALARM, 1, [Whether we have alarm()])
 fi
 
+AC_CHECK_HEADER([sys/mman.h],
+   [AC_DEFINE(HAVE_SYS_MMAN_H, [1], [Define to 1 if we have <sys/mman.h>])])
+
+AC_CHECK_FUNC(mprotect, have_mprotect=yes, have_mprotect=no)
+if test x$have_mprotect = xyes; then
+   AC_DEFINE(HAVE_MPROTECT, 1, [Whether we have mprotect()])
+fi
+
+AC_CHECK_FUNC(getpagesize, have_getpagesize=yes, have_getpagesize=no)
+if test x$have_getpagesize = xyes; then
+   AC_DEFINE(HAVE_GETPAGESIZE, 1, [Whether we have getpagesize()])
+fi
+
 dnl =====================================
 dnl Thread local storage
 
diff --git a/test/utils.c b/test/utils.c
index d95cbc2..ec6fd4e 100644
--- a/test/utils.c
+++ b/test/utils.c
@@ -5,6 +5,10 @@
 #include <unistd.h>
 #endif
 
+#ifdef HAVE_SYS_MMAN_H
+#include <sys/mman.h>
+#endif
+
 /* Random number seed
  */
 
@@ -197,10 +201,111 @@ image_endian_swap (pixman_image_t *img, int bpp)
     }
 }
 
+#define N_LEADING_PROTECTED	10
+#define N_TRAILING_PROTECTED	10
+#define INITIAL_PAGE
+
+typedef struct
+{
+    void *addr;
+    uint32_t len;
+    uint8_t *trailing;
+} info_t;
+
+#if defined(HAVE_MPROTECT) && defined(HAVE_GETPAGESIZE)
+
+void *
+fence_malloc (uint32_t len)
+{
+    unsigned long page_size = getpagesize();
+    unsigned long page_mask = page_size - 1;
+    uint32_t n_payload_bytes = (len + page_mask) & ~page_mask;
+    uint32_t n_bytes =
+	(len +
+	 page_size * (N_LEADING_PROTECTED + N_TRAILING_PROTECTED + 2) +
+	 n_payload_bytes) & ~page_mask;
+    uint8_t *initial_page;
+    uint8_t *leading_protected;
+    uint8_t *trailing_protected;
+    uint8_t *payload;
+    uint8_t *addr;
+
+    addr = malloc (n_bytes);
+
+    if (!addr)
+    {
+	printf ("malloc failed on %u %u\n", len, n_bytes);
+	return NULL;
+    }
+
+    initial_page = (uint8_t *)(((unsigned long)addr + page_mask) & ~page_mask);
+    leading_protected = initial_page + page_size;
+    payload = leading_protected + N_LEADING_PROTECTED * page_size;
+    trailing_protected = payload + n_payload_bytes;
+
+    ((info_t *)initial_page)->addr = addr;
+    ((info_t *)initial_page)->len = len;
+    ((info_t *)initial_page)->trailing = trailing_protected;
+
+    if (mprotect (leading_protected, N_LEADING_PROTECTED * page_size,
+		  PROT_NONE) == -1)
+    {
+	free (addr);
+	return NULL;
+    }
+
+    if (mprotect (trailing_protected, N_TRAILING_PROTECTED * page_size,
+		  PROT_NONE) == -1)
+    {
+	mprotect (leading_protected, N_LEADING_PROTECTED * page_size,
+		  PROT_READ | PROT_WRITE);
+
+	free (addr);
+	return NULL;
+    }
+
+    return payload;
+}
+
+void
+fence_free (void *data)
+{
+    uint32_t page_size = getpagesize();
+    uint8_t *payload = data;
+    uint8_t *leading_protected = payload - N_LEADING_PROTECTED * page_size;
+    uint8_t *initial_page = leading_protected - page_size;
+    info_t *info = (info_t *)initial_page;
+    uint8_t *trailing_protected = info->trailing;
+
+    mprotect (leading_protected, N_LEADING_PROTECTED * page_size,
+	      PROT_READ | PROT_WRITE);
+
+    mprotect (trailing_protected, N_LEADING_PROTECTED * page_size,
+	      PROT_READ | PROT_WRITE);
+
+    free (info->addr);
+}
+
+#else
+
+void *
+fence_malloc (uint32_t len)
+{
+    return malloc (len);
+}
+
+void
+fence_free (void *data)
+{
+    free (data);
+}
+
+#endif
+
 uint8_t *
 make_random_bytes (int n_bytes)
 {
-    uint8_t *bytes = malloc (n_bytes);
+    uint8_t *bytes = fence_malloc (n_bytes);
     int i;
 
     if (!bytes)
diff --git a/test/utils.h b/test/utils.h
index a39af02..14e3c8b 100644
--- a/test/utils.h
+++ b/test/utils.h
@@ -51,7 +51,16 @@ compute_crc32 (uint32_t    in_crc32,
 void
 image_endian_swap (pixman_image_t *img, int bpp);
 
-/* Generate n_bytes random bytes in malloced memory */
+/* Allocate memory that is bounded by protected pages,
+ * so that out-of-bounds access will cause segfaults
+ */
+void *
+fence_malloc (uint32_t len);
+
+void
+fence_free (void *data);
+
+/* Generate n_bytes random bytes in fence_malloced memory */
 uint8_t *
 make_random_bytes (int n_bytes);