Change the way malloc_init() works so that the main data structures

can be made immutable to provide extra protection.  Also init pools
on-demand: only pools that are actually used are initialized.

Tested by many

1 files changed, 66 insertions, 65 deletions

diff --git a/src/lib/libc/stdlib/malloc.c b/src/lib/libc/stdlib/malloc.c
index a0ee04f821..99249b24cb 100644
--- a/src/lib/libc/stdlib/malloc.c
+++ b/src/lib/libc/stdlib/malloc.c
@@ -1,4 +1,4 @@
-/*      $OpenBSD: malloc.c,v 1.275 2022/10/14 04:38:39 deraadt Exp $    */
+/*      $OpenBSD: malloc.c,v 1.276 2022/12/27 17:31:09 otto Exp $       */
 /*
  * Copyright (c) 2008, 2010, 2011, 2016 Otto Moerbeek <otto@drijf.net>
  * Copyright (c) 2012 Matthew Dempsky <matthew@openbsd.org>
@@ -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_t sz, void *f)
         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 *argpool, struct dir_info **foundpool,
         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 dir_info *d)
         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,