SEO

idea came to me working on something else, but it was quick and easy.
something to consider for after 3.6.

basically, the following mistake is pretty easy to make:
struct foo *f;
f = malloc(sizeof(f)); /* wrong! should be sizeof(*f) */
anil added the bounds attribute to gcc which helped catch many instances
of a similar bug with functions like memset and fixed size arrays. it's
harder to detect the bug with malloc. but we can due some runtime size
enforcement.

we already have guard pages. unfortunately, sizes are rounded up to 16
bytes, so small structs will never hit them. additionally, only the last
chunk of a page will overflow into the guard, the others overflow into the
next chunk. sizeof(void *) allocations are fairly rare, however, so we
can treat them specially.

when malloc gets a request with a size equal to the size of a pointer, we
can allocate a whole page, and return a pointer 4 bytes from the end. the
four bytes allocated are useable, but don't touch the fifth or any later
ones. (8 bytes on 64bit archs).

when running with this patch and malloc guard, any attempt to access
fields of foo after the first will cause the program containing the above
code to crash. typically just after the problem allocation. having
seen and fixed several bugs of this nature, i can say that they're
fairly prevalent. happy hunting.

Index: malloc.c
================================================== =================
RCS file: /cvs/src/lib/libc/stdlib/malloc.c,v
retrieving revision 1.71
diff -u -r1.71 malloc.c
--- malloc.c 11 Aug 2004 06:22:45 -0000 1.71
+++ malloc.c 13 Sep 2004 04:29:36 -0000
@@ -858,12 +858,20 @@
}

/*
+ * magic so that malloc(sizeof(ptr)) is near the end of the page.
+ */
+#define PTR_GAP (malloc_pagesize - sizeof(void *))
+#define PTR_SIZE (sizeof(void *))
+#define PTR_ALIGNED(p) (((unsigned long)p & malloc_pagemask) == PTR_GAP)
+
+/*
* Allocate a piece of memory
*/
static void *
imalloc(size_t size)
{
void *result;
+ int ptralloc = 0;

if (!malloc_started)
malloc_init();
@@ -871,6 +879,11 @@
if (suicide)
abort();

+ if (malloc_guard && size == PTR_SIZE) {
+ ptralloc = 1;
+ size = malloc_pagesize;
+ }
+
if ((size + malloc_pagesize) < size) { /* Check for overflow */
result = NULL;
errno = ENOMEM;
@@ -886,6 +899,8 @@
if (malloc_zero && result != NULL)
memset(result, 0, size);

+ if (result && ptralloc)
+ return ((char *)result + PTR_GAP);
return (result);
}

@@ -908,6 +923,19 @@
return (NULL);
}

+ if (malloc_guard && PTR_ALIGNED(ptr)) {
+ if (size <= PTR_SIZE)
+ return (ptr);
+ else {
+ p = imalloc(size);
+ if (p)
+ memcpy(p, ptr, PTR_SIZE);
+ ifree(ptr);
+ return (p);
+ }
+ }
+
+
index = ptr2index(ptr);

if (index < malloc_pageshift) {
@@ -1225,6 +1253,9 @@
/* If we're already sinking, don't make matters any worse. */
if (suicide)
return;
+
+ if (malloc_guard && PTR_ALIGNED(ptr))
+ ptr = (char *)ptr - PTR_GAP;

index = ptr2index(ptr);



--
ask not what you can do for your country
ask what your country did to you