SEO

On Mon, 13 Sep 2004, Ted Unangst wrote:

step back in time. i think it's time for this to happen. updated diff
below.

> 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.72
diff -u -r1.72 malloc.c
--- malloc.c 31 Mar 2005 21:24:46 -0000 1.72
+++ malloc.c 24 May 2005 07:04:38 -0000
@@ -1061,6 +1061,13 @@
}

/*
+ * 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 *
@@ -1075,6 +1082,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;
@@ -1090,6 +1102,8 @@
if (malloc_zero && result != NULL)
memset(result, 0, size);

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

@@ -1114,6 +1128,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) {
@@ -1574,6 +1601,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);


--
we fear that pop-culture
is the only kind of culture we're ever going to have