Re: [dpb@infoseek.com] libc/855: calloc returns non-zeroed memory

[Wolfram Gloger <wmglo@dent.med.uni-muenchen.de>]

Hello,

This is a rather critical malloc bug: multi-threaded (and only
multi-threaded) programs sometimes get non-zeroed memory from
calloc().  Many thanks to dpb@infoseek.com for observing and reporting
this bug.

Also, this is entirely my fault because of a horrible misunderstanding
about the way mprotect() works (it never discards any memory, even
when you request PROT_NONE...)  I feel quite stupid now.  Sorry !

Please apply the following patch to glibc-2.0.x (I'm so glad 2.0.8
isn't released yet...).  For 2.1.x all that should possibly be changed
in the patch is to check the mmap() result for MAP_FAILED rather than
(char *)-1, like it is done consistently in that branch's malloc.c if
I remember right.

Regards,
Wolfram.

--- malloc.c	1998/05/12 15:44:22	1.1.1.3
+++ malloc.c	1998/11/04 16:56:02
@@ -1766,15 +1766,15 @@
 
 static int dev_zero_fd = -1; /* Cached file descriptor for /dev/zero. */
 
-#define MMAP(size, prot, flags) ((dev_zero_fd < 0) ? \
+#define MMAP(addr, size, prot, flags) ((dev_zero_fd < 0) ? \
  (dev_zero_fd = open("/dev/zero", O_RDWR), \
-  mmap(0, (size), (prot), (flags), dev_zero_fd, 0)) : \
-   mmap(0, (size), (prot), (flags), dev_zero_fd, 0))
+  mmap((addr), (size), (prot), (flags), dev_zero_fd, 0)) : \
+   mmap((addr), (size), (prot), (flags), dev_zero_fd, 0))
 
 #else
 
-#define MMAP(size, prot, flags) \
- (mmap(0, (size), (prot), (flags)|MAP_ANONYMOUS, -1, 0))
+#define MMAP(addr, size, prot, flags) \
+ (mmap((addr), (size), (prot), (flags)|MAP_ANONYMOUS, -1, 0))
 
 #endif
 
@@ -1794,7 +1794,7 @@
    */
   size = (size + SIZE_SZ + page_mask) & ~page_mask;
 
-  p = (mchunkptr)MMAP(size, PROT_READ|PROT_WRITE, MAP_PRIVATE);
+  p = (mchunkptr)MMAP(0, size, PROT_READ|PROT_WRITE, MAP_PRIVATE);
   if(p == (mchunkptr)-1) return 0;
 
   n_mmaps++;
@@ -1926,7 +1926,7 @@
      No swap space needs to be reserved for the following large
      mapping (on Linux, this is the case for all non-writable mappings
      anyway). */
-  p1 = (char *)MMAP(HEAP_MAX_SIZE<<1, PROT_NONE, MAP_PRIVATE|MAP_NORESERVE);
+  p1 = (char *)MMAP(0, HEAP_MAX_SIZE<<1, PROT_NONE, MAP_PRIVATE|MAP_NORESERVE);
   if(p1 == (char *)-1)
     return 0;
   p2 = (char *)(((unsigned long)p1 + HEAP_MAX_SIZE) & ~(HEAP_MAX_SIZE-1));
@@ -1967,7 +1967,10 @@
     new_size = (long)h->size + diff;
     if(new_size < (long)sizeof(*h))
       return -1;
-    if(mprotect((char *)h + new_size, -diff, PROT_NONE) != 0)
+    /* Try to re-map the extra heap space freshly to save memory, and
+       make it inaccessible. */
+    if((char *)MMAP((char *)h + new_size, -diff, PROT_NONE,
+                    MAP_PRIVATE|MAP_FIXED) == (char *)-1)
       return -2;
   }
   h->size = new_size;
@@ -2122,7 +2125,10 @@
   if(ar_ptr != &main_arena) {
     heap_info *heap = heap_for_ptr(p);
     assert(heap->ar_ptr == ar_ptr);
-    assert((char *)p + sz <= (char *)heap + heap->size);
+    if(p != top(ar_ptr))
+      assert((char *)p + sz <= (char *)heap + heap->size);
+    else
+      assert((char *)p + sz == (char *)heap + heap->size);
     return;
   }
 #endif