From eead62184ddf7a37794b2d6bac83643be7180c7a Mon Sep 17 00:00:00 2001 From: tdeval <> Date: Thu, 7 Jul 2005 05:28:53 +0000 Subject: [PATCH] Fix the unmapping of freed pages, leaving just 64k worth of cache pages. Prodded by art@ and fgsch@, ok deraadt@ --- src/lib/libc/stdlib/malloc.c | 194 ++++++++++++++++++++++++----------- 1 file changed, 136 insertions(+), 58 deletions(-) diff --git a/src/lib/libc/stdlib/malloc.c b/src/lib/libc/stdlib/malloc.c index e3405df3..5a328b0e 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_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)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; }