On Sun, Dec 18, 2022 at 08:10:48PM +0100, Otto Moerbeek wrote: > Hi, > > This is the latest verion of a diff I sent around previously, but now > it's time this gets wider testing for real. > > The main purpose is to rearrange malloc init in such a way that a few > pages containing mallic internal data structures can be made > immutable. In addtion to that, each pools is inited on-demand instead > of always. > > So please test! I did not get a lot of feedback on the earlier versions.
Many thanks to all who tested so far! I do lacks test reports on some more exoctic platforms, these days that is !(amd64 || arm64). Any takers? -Otto > > Index: stdlib/malloc.c > =================================================================== > RCS file: /home/cvs/src/lib/libc/stdlib/malloc.c,v > retrieving revision 1.275 > diff -u -p -r1.275 malloc.c > --- stdlib/malloc.c 14 Oct 2022 04:38:39 -0000 1.275 > +++ stdlib/malloc.c 18 Dec 2022 18:57:16 -0000 > @@ -142,6 +142,7 @@ struct dir_info { > int malloc_junk; /* junk fill? */ > int mmap_flag; /* extra flag for mmap */ > int mutex; > + int malloc_mt; /* multi-threaded mode? */ > /* lists of free chunk info structs */ > struct chunk_head chunk_info_list[MALLOC_MAXSHIFT + 1]; > /* lists of chunks with free slots */ > @@ -181,8 +182,6 @@ struct dir_info { > #endif /* MALLOC_STATS */ > u_int32_t canary2; > }; > -#define DIR_INFO_RSZ ((sizeof(struct dir_info) + MALLOC_PAGEMASK) & \ > - ~MALLOC_PAGEMASK) > > static void unmap(struct dir_info *d, void *p, size_t sz, size_t clear); > > @@ -208,7 +207,6 @@ struct malloc_readonly { > /* Main bookkeeping information */ > struct dir_info *malloc_pool[_MALLOC_MUTEXES]; > u_int malloc_mutexes; /* how much in actual use? */ > - int malloc_mt; /* multi-threaded mode? */ > int malloc_freecheck; /* Extensive double free check */ > int malloc_freeunmap; /* mprotect free pages PROT_NONE? */ > int def_malloc_junk; /* junk fill? */ > @@ -258,7 +256,7 @@ static void malloc_exit(void); > static inline void > _MALLOC_LEAVE(struct dir_info *d) > { > - if (mopts.malloc_mt) { > + if (d->malloc_mt) { > d->active--; > _MALLOC_UNLOCK(d->mutex); > } > @@ -267,7 +265,7 @@ _MALLOC_LEAVE(struct dir_info *d) > static inline void > _MALLOC_ENTER(struct dir_info *d) > { > - if (mopts.malloc_mt) { > + if (d->malloc_mt) { > _MALLOC_LOCK(d->mutex); > d->active++; > } > @@ -292,7 +290,7 @@ hash(void *p) > static inline struct dir_info * > getpool(void) > { > - if (!mopts.malloc_mt) > + if (mopts.malloc_pool[1] == NULL || !mopts.malloc_pool[1]->malloc_mt) > return mopts.malloc_pool[1]; > else /* first one reserved for special pool */ > return mopts.malloc_pool[1 + TIB_GET()->tib_tid % > @@ -497,46 +495,22 @@ omalloc_init(void) > } > > static void > -omalloc_poolinit(struct dir_info **dp, int mmap_flag) > +omalloc_poolinit(struct dir_info *d, int mmap_flag) > { > - char *p; > - size_t d_avail, regioninfo_size; > - struct dir_info *d; > int i, j; > > - /* > - * Allocate dir_info with a guard page on either side. Also > - * randomise offset inside the page at which the dir_info > - * lies (subject to alignment by 1 << MALLOC_MINSHIFT) > - */ > - if ((p = MMAPNONE(DIR_INFO_RSZ + (MALLOC_PAGESIZE * 2), mmap_flag)) == > - MAP_FAILED) > - wrterror(NULL, "malloc init mmap failed"); > - mprotect(p + MALLOC_PAGESIZE, DIR_INFO_RSZ, PROT_READ | PROT_WRITE); > - d_avail = (DIR_INFO_RSZ - sizeof(*d)) >> MALLOC_MINSHIFT; > - d = (struct dir_info *)(p + MALLOC_PAGESIZE + > - (arc4random_uniform(d_avail) << MALLOC_MINSHIFT)); > - > - rbytes_init(d); > - d->regions_free = d->regions_total = MALLOC_INITIAL_REGIONS; > - regioninfo_size = d->regions_total * sizeof(struct region_info); > - d->r = MMAP(regioninfo_size, mmap_flag); > - if (d->r == MAP_FAILED) { > - d->regions_total = 0; > - wrterror(NULL, "malloc init mmap failed"); > - } > + d->r = NULL; > + d->rbytesused = sizeof(d->rbytes); > + d->regions_free = d->regions_total = 0; > for (i = 0; i <= MALLOC_MAXSHIFT; i++) { > LIST_INIT(&d->chunk_info_list[i]); > for (j = 0; j < MALLOC_CHUNK_LISTS; j++) > LIST_INIT(&d->chunk_dir[i][j]); > } > - STATS_ADD(d->malloc_used, regioninfo_size + 3 * MALLOC_PAGESIZE); > d->mmap_flag = mmap_flag; > d->malloc_junk = mopts.def_malloc_junk; > d->canary1 = mopts.malloc_canary ^ (u_int32_t)(uintptr_t)d; > d->canary2 = ~d->canary1; > - > - *dp = d; > } > > static int > @@ -551,7 +525,8 @@ omalloc_grow(struct dir_info *d) > if (d->regions_total > SIZE_MAX / sizeof(struct region_info) / 2) > return 1; > > - newtotal = d->regions_total * 2; > + newtotal = d->regions_total == 0 ? MALLOC_INITIAL_REGIONS : > + d->regions_total * 2; > newsize = PAGEROUND(newtotal * sizeof(struct region_info)); > mask = newtotal - 1; > > @@ -576,10 +551,12 @@ omalloc_grow(struct dir_info *d) > } > } > > - oldpsz = PAGEROUND(d->regions_total * sizeof(struct region_info)); > - /* clear to avoid meta info ending up in the cache */ > - unmap(d, d->r, oldpsz, oldpsz); > - d->regions_free += d->regions_total; > + if (d->regions_total > 0) { > + oldpsz = PAGEROUND(d->regions_total * sizeof(struct > region_info)); > + /* clear to avoid meta info ending up in the cache */ > + unmap(d, d->r, oldpsz, oldpsz); > + } > + d->regions_free += newtotal - d->regions_total; > d->regions_total = newtotal; > d->r = p; > return 0; > @@ -596,7 +573,7 @@ insert(struct dir_info *d, void *p, size > size_t mask; > void *q; > > - if (d->regions_free * 4 < d->regions_total) { > + if (d->regions_free * 4 < d->regions_total || d->regions_total == 0) { > if (omalloc_grow(d)) > return 1; > } > @@ -628,6 +605,8 @@ find(struct dir_info *d, void *p) > if (mopts.malloc_canary != (d->canary1 ^ (u_int32_t)(uintptr_t)d) || > d->canary1 != ~d->canary2) > wrterror(d, "internal struct corrupt"); > + if (d->r == NULL) > + return NULL; > p = MASK_POINTER(p); > index = hash(p) & mask; > r = d->r[index].p; > @@ -1300,18 +1279,50 @@ _malloc_init(int from_rthreads) > _MALLOC_UNLOCK(1); > return; > } > - if (!mopts.malloc_canary) > + if (!mopts.malloc_canary) { > + char *p; > + size_t sz, d_avail; > + > omalloc_init(); > + /* > + * Allocate dir_infos with a guard page on either side. Also > + * randomise offset inside the page at which the dir_infos > + * lay (subject to alignment by 1 << MALLOC_MINSHIFT) > + */ > + sz = mopts.malloc_mutexes * sizeof(*d) + 2 * MALLOC_PAGESIZE; > + if ((p = MMAPNONE(sz, 0)) == MAP_FAILED) > + wrterror(NULL, "malloc_init mmap1 failed"); > + if (mprotect(p + MALLOC_PAGESIZE, mopts.malloc_mutexes * > sizeof(*d), > + PROT_READ | PROT_WRITE)) > + wrterror(NULL, "malloc_init mprotect1 failed"); > + if (mimmutable(p, sz)) > + wrterror(NULL, "malloc_init mimmutable1 failed"); > + d_avail = (((mopts.malloc_mutexes * sizeof(*d) + > MALLOC_PAGEMASK) & > + ~MALLOC_PAGEMASK) - (mopts.malloc_mutexes * sizeof(*d))) >> > + MALLOC_MINSHIFT; > + d = (struct dir_info *)(p + MALLOC_PAGESIZE + > + (arc4random_uniform(d_avail) << MALLOC_MINSHIFT)); > + STATS_ADD(d[1].malloc_used, sz); > + for (i = 0; i < mopts.malloc_mutexes; i++) > + mopts.malloc_pool[i] = &d[i]; > + mopts.internal_funcs = 1; > + if (((uintptr_t)&malloc_readonly & MALLOC_PAGEMASK) == 0) { > + if (mprotect(&malloc_readonly, sizeof(malloc_readonly), > + PROT_READ)) > + wrterror(NULL, "malloc_init mprotect r/o > failed"); > + if (mimmutable(&malloc_readonly, > sizeof(malloc_readonly))) > + wrterror(NULL, "malloc_init mimmutable r/o > failed"); > + } > + } > > nmutexes = from_rthreads ? mopts.malloc_mutexes : 2; > - if (((uintptr_t)&malloc_readonly & MALLOC_PAGEMASK) == 0) > - mprotect(&malloc_readonly, sizeof(malloc_readonly), > - PROT_READ | PROT_WRITE); > for (i = 0; i < nmutexes; i++) { > - if (mopts.malloc_pool[i]) > + d = mopts.malloc_pool[i]; > + d->malloc_mt = from_rthreads; > + if (d->canary1 == ~d->canary2) > continue; > if (i == 0) { > - omalloc_poolinit(&d, MAP_CONCEAL); > + omalloc_poolinit(d, MAP_CONCEAL); > d->malloc_junk = 2; > d->bigcache_size = 0; > for (j = 0; j < MAX_SMALLCACHEABLE_SIZE; j++) > @@ -1319,7 +1330,7 @@ _malloc_init(int from_rthreads) > } else { > size_t sz = 0; > > - omalloc_poolinit(&d, 0); > + omalloc_poolinit(d, 0); > d->malloc_junk = mopts.def_malloc_junk; > d->bigcache_size = mopts.def_maxcache; > for (j = 0; j < MAX_SMALLCACHEABLE_SIZE; j++) { > @@ -1332,7 +1343,9 @@ _malloc_init(int from_rthreads) > void *p = MMAP(sz, 0); > if (p == MAP_FAILED) > wrterror(NULL, > - "malloc init mmap failed"); > + "malloc init mmap2 failed"); > + if (mimmutable(p, sz)) > + wrterror(NULL, "malloc_init mimmutable2 > failed"); > for (j = 0; j < MAX_SMALLCACHEABLE_SIZE; j++) { > d->smallcache[j].pages = p; > p = (char *)p + d->smallcache[j].max * > @@ -1342,20 +1355,8 @@ _malloc_init(int from_rthreads) > } > } > d->mutex = i; > - mopts.malloc_pool[i] = d; > } > > - if (from_rthreads) > - mopts.malloc_mt = 1; > - else > - mopts.internal_funcs = 1; > - > - /* > - * Options have been set and will never be reset. > - * Prevent further tampering with them. > - */ > - if (((uintptr_t)&malloc_readonly & MALLOC_PAGEMASK) == 0) > - mprotect(&malloc_readonly, sizeof(malloc_readonly), PROT_READ); > _MALLOC_UNLOCK(1); > } > DEF_STRONG(_malloc_init); > @@ -1420,7 +1421,7 @@ findpool(void *p, struct dir_info *argpo > if (r == NULL) { > u_int i, nmutexes; > > - nmutexes = mopts.malloc_mt ? mopts.malloc_mutexes : 2; > + nmutexes = mopts.malloc_pool[1]->malloc_mt ? > mopts.malloc_mutexes : 2; > STATS_INC(pool->other_pool); > for (i = 1; i < nmutexes; i++) { > u_int j = (argpool->mutex + i) & (nmutexes - 1); > @@ -2332,7 +2333,7 @@ malloc_dump1(int fd, int poolno, struct > dprintf(fd, "Malloc dir of %s pool %d at %p\n", __progname, poolno, d); > if (d == NULL) > return; > - dprintf(fd, "J=%d Fl=%x\n", d->malloc_junk, d->mmap_flag); > + dprintf(fd, "MT=%d J=%d Fl=%x\n", d->malloc_mt, d->malloc_junk, > d->mmap_flag); > dprintf(fd, "Region slots free %zu/%zu\n", > d->regions_free, d->regions_total); > dprintf(fd, "Finds %zu/%zu\n", d->finds, d->find_collisions); > @@ -2421,9 +2422,9 @@ malloc_exit(void) > if (fd != -1) { > dprintf(fd, "******** Start dump %s *******\n", __progname); > dprintf(fd, > - "MT=%d M=%u I=%d F=%d U=%d J=%d R=%d X=%d C=%d cache=%u " > + "M=%u I=%d F=%d U=%d J=%d R=%d X=%d C=%d cache=%u " > "G=%zu\n", > - mopts.malloc_mt, mopts.malloc_mutexes, > + mopts.malloc_mutexes, > mopts.internal_funcs, mopts.malloc_freecheck, > mopts.malloc_freeunmap, mopts.def_malloc_junk, > mopts.malloc_realloc, mopts.malloc_xmalloc, > >