Fix the unmapping of freed pages, leaving just 64k worth of cache pages.

Prodded by art@ and fgsch@, ok deraadt@

1 files changed, 136 insertions, 58 deletions

diff --git a/src/lib/libc/stdlib/malloc.c b/src/lib/libc/stdlib/malloc.c
index e3405df39a..5a328b0eb0 100644
--- a/src/lib/libc/stdlib/malloc.c
+++ b/src/lib/libc/stdlib/malloc.c
@@ -8,7 +8,7 @@
  */
 
 #if defined(LIBC_SCCS) && !defined(lint)
-static char rcsid[] = "$OpenBSD: malloc.c,v 1.74 2005/06/07 04:42:42 tedu Exp $";
+static char rcsid[] = "$OpenBSD: malloc.c,v 1.75 2005/07/07 05:28:53 tdeval Exp $";
 #endif /* LIBC_SCCS and not lint */
 
 /*
@@ -142,8 +142,9 @@ struct pgfree {
 /* A mask for the offset inside a page.  */
 #define malloc_pagemask ((malloc_pagesize)-1)
 
-#define pageround(foo) (((foo) + (malloc_pagemask)) & ~malloc_pagemask)
-#define ptr2index(foo) (((u_long)(foo) >> malloc_pageshift)+malloc_pageshift)
+#define pageround(foo)  (((foo) + (malloc_pagemask)) & ~malloc_pagemask)
+#define ptr2index(foo)  (((u_long)(foo) >> malloc_pageshift)+malloc_pageshift)
+#define index2ptr(idx)  ((void*)(((idx)-malloc_pageshift)<<malloc_pageshift))
 
 /* fd of /dev/zero */
 #ifdef USE_DEV_ZERO
@@ -211,6 +212,7 @@ static int malloc_freeprot;
 
 /* use guard pages after allocations? */
 static int malloc_guard = 0;
+static int malloc_guarded;
 /* align pointers to end of page? */
 static int malloc_ptrguard;
 
@@ -396,6 +398,7 @@ malloc_dump(FILE *fd)
     fprintf(fd, "Pagesize\t%lu\n", (u_long)malloc_pagesize);
     fprintf(fd, "Pageshift\t%d\n", malloc_pageshift);
     fprintf(fd, "In use\t%lu\n", (u_long)malloc_used);
+    fprintf(fd, "Guarded\t%lu\n", (u_long)malloc_guarded);
 }
 #endif  /* MALLOC_STATS */
 
@@ -472,9 +475,9 @@ map_pages(size_t pages)
 {
     struct pdinfo *pi, *spi;
     struct pginfo **pd;
-    u_long pidx,lidx;
+    u_long idx, pidx, lidx;
     void *result, *tail;
-    u_long index;
+    u_long index, lindex;
 
     pages <<= malloc_pageshift;
     result = MMAP(pages + malloc_guard);
@@ -485,23 +488,25 @@ map_pages(size_t pages)
 #endif  /* MALLOC_EXTRA_SANITY */
         return (NULL);
     }
+    index = ptr2index(result);
     tail = result + pages + malloc_guard;
+    lindex = ptr2index(tail) - 1;
     if (malloc_guard)
         mprotect(result + pages, malloc_guard, PROT_NONE);
 
-    if (tail > malloc_brk)
+    pidx = PI_IDX(index);
+    lidx = PI_IDX(lindex);
+
+    if (tail > malloc_brk) {
         malloc_brk = tail;
-    if ((index = ptr2index(tail) - 1) > last_index)
-        last_index = index;
+        last_index = lindex;
+    }
 
     /* Insert directory pages, if needed. */
-    pidx = PI_IDX(ptr2index(result));
-    lidx = PI_IDX(index);
-
-    pdir_lookup(ptr2index(result), &pi);
+    pdir_lookup(index, &pi);
 
-    for (index=pidx,spi=pi;index<=lidx;index++) {
-        if (pi == NULL || PD_IDX(pi->dirnum) != index) {
+    for (idx=pidx,spi=pi;idx<=lidx;idx++) {
+        if (pi == NULL || PD_IDX(pi->dirnum) != idx) {
             if ((pd = MMAP(malloc_pagesize)) == MAP_FAILED) {
                 errno = ENOMEM;
                 munmap(result, tail - result);
@@ -515,31 +520,31 @@ map_pages(size_t pages)
             pi->base = pd;
             pi->prev = spi;
             pi->next = spi->next;
-            pi->dirnum = index * (malloc_pagesize/sizeof(struct pginfo *));
+            pi->dirnum = idx * (malloc_pagesize/sizeof(struct pginfo *));
 
             if (spi->next != NULL)
                 spi->next->prev = pi;
             spi->next = pi;
         }
-        if (index > pidx && index < lidx) {
+        if (idx > pidx && idx < lidx) {
             pi->dirnum += pdi_mod;
-        } else if (index == pidx) {
+        } else if (idx == pidx) {
             if (pidx == lidx) {
                 pi->dirnum += (tail - result) >> malloc_pageshift;
             } else {
-                pi->dirnum += pdi_mod - PI_OFF(ptr2index(result));
+                pi->dirnum += pdi_mod - PI_OFF(index);
             }
         } else {
             pi->dirnum += PI_OFF(ptr2index(tail - 1)) + 1;
         }
 #ifdef  MALLOC_EXTRA_SANITY
-        if (PD_OFF(pi->dirnum) > pdi_mod || PD_IDX(pi->dirnum) > index) {
+        if (PD_OFF(pi->dirnum) > pdi_mod || PD_IDX(pi->dirnum) > idx) {
             wrterror("(ES): pages directory overflow\n");
             errno = EFAULT;
             return (NULL);
         }
 #endif  /* MALLOC_EXTRA_SANITY */
-        if (index == pidx && pi != last_dir) {
+        if (idx == pidx && pi != last_dir) {
            prev_dir = last_dir;
            last_dir = pi;
         }
@@ -852,6 +857,7 @@ malloc_pages(size_t size)
         }
 
         malloc_used += size << malloc_pageshift;
+        malloc_guarded += malloc_guard;
 
         if (malloc_junk)
             memset(p, SOME_JUNK, size << malloc_pageshift);
@@ -1065,11 +1071,11 @@ malloc_bytes(size_t size)
 }
 
 /*
- * magic so that malloc(sizeof(ptr)) is near the end of the page.
+ * 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)
+#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
@@ -1087,8 +1093,8 @@ imalloc(size_t size)
         abort();
 
     if (malloc_ptrguard && size == PTR_SIZE) {
-            ptralloc = 1;
-            size = malloc_pagesize;
+        ptralloc = 1;
+        size = malloc_pagesize;
     }
 
     if ((size + malloc_pagesize) < size) {     /* Check for overflow */
@@ -1107,7 +1113,7 @@ imalloc(size_t size)
         memset(result, 0, size);
 
     if (result && ptralloc)
-            return ((char *)result + PTR_GAP);
+        return ((char *)result + PTR_GAP);
     return (result);
 }
 
@@ -1133,18 +1139,17 @@ irealloc(void *ptr, size_t size)
     }
 
     if (malloc_ptrguard && PTR_ALIGNED(ptr)) {
-            if (size <= PTR_SIZE)
-                    return (ptr);
-            else {
-                    p = imalloc(size);
-                    if (p)
-                            memcpy(p, ptr, PTR_SIZE);
-                    ifree(ptr);
-                    return (p);
-            }
+        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) {
@@ -1240,7 +1245,7 @@ irealloc(void *ptr, size_t size)
         }
 
     } else {
-        wrtwarning("pointer to wrong page\n");
+        wrtwarning("irealloc: pointer to wrong page\n");
         return (NULL);
     }
 
@@ -1268,7 +1273,7 @@ irealloc(void *ptr, size_t size)
 static __inline__ void
 free_pages(void *ptr, u_long index, struct pginfo *info)
 {
-    u_long i, l;
+    u_long i, l, cachesize = 0;
     struct pginfo **pd;
     struct pdinfo *pi, *spi;
     u_long pidx, lidx;
@@ -1281,7 +1286,7 @@ free_pages(void *ptr, u_long index, struct pginfo *info)
     }
 
     if (info != MALLOC_FIRST) {
-        wrtwarning("pointer to wrong page\n");
+        wrtwarning("free_pages: pointer to wrong page\n");
         return;
     }
 
@@ -1330,6 +1335,7 @@ free_pages(void *ptr, u_long index, struct pginfo *info)
         memset(ptr, SOME_JUNK, l);
 
     malloc_used -= l;
+    malloc_guarded -= malloc_guard;
     if (malloc_guard) {
 #ifdef  MALLOC_EXTRA_SANITY
         if (pi == NULL || PD_IDX(pi->dirnum) != PI_IDX(index+i)) {
@@ -1371,9 +1377,18 @@ free_pages(void *ptr, u_long index, struct pginfo *info)
 
         /* Find the right spot, leave pf pointing to the modified entry. */
 
-        for(pf = free_list.next; (pf->page+pf->size) < ptr && pf->next != NULL;
-            pf = pf->next)
-            ; /* Race ahead here. */
+        /* Race ahead here, while calculating cache size. */
+        for (pf = free_list.next;
+             (pf->page + pf->size) < ptr && pf->next != NULL;
+             pf = pf->next)
+                cachesize += pf->size;
+
+        /* Finish cache size calculation. */
+        pt = pf;
+        while (pt) {
+            cachesize += pt->size;
+            pt = pt->next;
+        }
 
         if (pf->page > tail) {
             /* Insert before entry */
@@ -1385,6 +1400,7 @@ free_pages(void *ptr, u_long index, struct pginfo *info)
             px = NULL;
         } else if ((pf->page + pf->size) == ptr ) {
             /* Append to the previous entry. */
+            cachesize -= pf->size;
             pf->size += l;
             if (pf->next != NULL && (pf->page + pf->size) == pf->next->page ) {
                 /* And collapse the next too. */
@@ -1396,6 +1412,7 @@ free_pages(void *ptr, u_long index, struct pginfo *info)
             }
         } else if (pf->page == tail) {
             /* Prepend to entry. */
+            cachesize -= pf->size;
             pf->size += l;
             pf->page = ptr;
             pf->pdir = spi;
@@ -1419,34 +1436,32 @@ free_pages(void *ptr, u_long index, struct pginfo *info)
     }
 
     /* Return something to OS ? */
-    if (pf->next == NULL &&                     /* If we're the last one, */
-      pf->size > malloc_cache &&                /* ..and the cache is full, */
-      (pf->page + pf->size) == malloc_brk) {    /* ..and none behind us, */
+    if (pf->size > (malloc_cache - cachesize)) {
 
         /*
          * Keep the cache intact.  Notice that the '>' above guarantees that
          * the pf will always have at least one page afterwards.
          */
-        if (munmap((char *)pf->page + malloc_cache, pf->size - malloc_cache)!=0)
+        if (munmap((char *)pf->page + (malloc_cache - cachesize),
+                   pf->size - (malloc_cache - cachesize)) != 0)
             goto not_return;
         tail = pf->page + pf->size;
         lidx = ptr2index(tail) - 1;
-        pf->size = malloc_cache;
+        pf->size = malloc_cache - cachesize;
 
-        malloc_brk = pf->page + malloc_cache;
-
-        index = ptr2index(malloc_brk);
+        index = ptr2index(pf->page + pf->size);
 
         pidx = PI_IDX(index);
         if (prev_dir != NULL && PD_IDX(prev_dir->dirnum) >= pidx)
-                prev_dir = NULL;        /* Will be wiped out below ! */
+            prev_dir = NULL;    /* Will be wiped out below ! */
 
         for (pi=pf->pdir; pi!=NULL && PD_IDX(pi->dirnum)<pidx; pi=pi->next);
 
+        spi = pi;
         if (pi != NULL && PD_IDX(pi->dirnum) == pidx) {
             pd = pi->base;
 
-            for(i=index;i <= last_index;) {
+            for(i=index;i <= lidx;) {
                 if (pd[PI_OFF(i)] != MALLOC_NOT_MINE) {
                     pd[PI_OFF(i)] = MALLOC_NOT_MINE;
 #ifdef  MALLOC_EXTRA_SANITY
@@ -1458,12 +1473,19 @@ free_pages(void *ptr, u_long index, struct pginfo *info)
 #endif  /* MALLOC_EXTRA_SANITY */
                     pi->dirnum--;
                 }
+#ifdef  MALLOC_EXTRA_SANITY
+                else
+                    wrtwarning("(ES): page already unmapped\n");
+#endif  /* MALLOC_EXTRA_SANITY */
                 i++;
                 if (!PI_OFF(i)) {
                     /* If no page in that dir, free directory page. */
                     if (!PD_OFF(pi->dirnum)) {
                         /* Remove from list. */
-                        pi->prev->next = pi->next;
+                        if (spi == pi)  /* Update spi only if first. */
+                            spi = pi->prev;
+                        if (pi->prev != NULL)
+                            pi->prev->next = pi->next;
                         if (pi->next != NULL)
                             pi->next->prev = pi->prev;
                         pi = pi->next;
@@ -1475,11 +1497,65 @@ free_pages(void *ptr, u_long index, struct pginfo *info)
                     pd = pi->base;
                 }
             }
+            if (pi && !PD_OFF(pi->dirnum)) {
+                /* Resulting page dir is now empty. */
+                /* Remove from list. */
+                if (spi == pi)  /* Update spi only if first. */
+                    spi = pi->prev;
+                if (pi->prev != NULL)
+                    pi->prev->next = pi->next;
+                if (pi->next != NULL)
+                    pi->next->prev = pi->prev;
+                pi = pi->next;
+                munmap(pd, malloc_pagesize);
+            }
         }
 
-        last_index = index - 1;
+        if (pi == NULL && malloc_brk == tail) {
+            /* Resize down the malloc upper boundary. */
+            last_index = index - 1;
+            malloc_brk = index2ptr(index);
+        }
 
         /* XXX: We could realloc/shrink the pagedir here I guess. */
+        if (pf->size == 0) {    /* Remove from free-list as well. */
+            if (px)
+                ifree(px);
+            if ((px = pf->prev) != &free_list) {
+                if (pi == NULL && last_index == (index - 1)) {
+                    if (spi == NULL) {
+                        malloc_brk = NULL;
+                        i = 11;
+                    } else {
+                        pd = spi->base;
+                        if (PD_IDX(spi->dirnum) < pidx)
+                            index = ((PD_IDX(spi->dirnum) + 1) * pdi_mod) - 1;
+                        for (pi=spi,i=index;pd[PI_OFF(i)]==MALLOC_NOT_MINE;i--)
+#ifdef  MALLOC_EXTRA_SANITY
+                            if (!PI_OFF(i)) {   /* Should never enter here. */
+                                pi = pi->prev;
+                                if (pi == NULL || i == 0)
+                                    break;
+                                pd = pi->base;
+                                i = (PD_IDX(pi->dirnum) + 1) * pdi_mod;
+                            }
+#else   /* !MALLOC_EXTRA_SANITY */
+                            { }
+#endif  /* MALLOC_EXTRA_SANITY */
+                        malloc_brk = index2ptr(i + 1);
+                    }
+                    last_index = i;
+                }
+                if ((px->next = pf->next) != NULL)
+                    px->next->prev = px;
+            } else {
+                if ((free_list.next = pf->next) != NULL)
+                    free_list.next->prev = &free_list;
+            }
+            px = pf;
+            last_dir = prev_dir;
+            prev_dir = NULL;
+        }
     }
 not_return:
     if (pt != NULL)
@@ -1607,17 +1683,19 @@ ifree(void *ptr)
         return;
 
     if (malloc_ptrguard && PTR_ALIGNED(ptr))
-            ptr = (char *)ptr - PTR_GAP;
+        ptr = (char *)ptr - PTR_GAP;
 
     index = ptr2index(ptr);
 
     if (index < malloc_pageshift) {
-        wrtwarning("junk pointer, too low to make sense\n");
+        warnx("(%p)", ptr);
+        wrtwarning("ifree: junk pointer, too low to make sense\n");
         return;
     }
 
     if (index > last_index) {
-        wrtwarning("junk pointer, too high to make sense\n");
+        warnx("(%p)", ptr);
+        wrtwarning("ifree: junk pointer, too high to make sense\n");
         return;
     }