LinuxThreads' stacks can collide with malloc

[Wolfram Gloger <wmglo at dent dot med dot uni-muenchen dot de>]

Hello,

I've seen strange crashes in multi-threaded applications that almost
completely fill up the available address space (with malloc or
anonymous mmap()) on i586.

I am confident now that the problems are caused by LinuxThreads' use
of mmap(... MAP_FIXED) to allocate the thread stacks.  The stacks are
unmapped at thread exit time, so if a stack region is later occupied
by e.g. malloc, and still later another thread is started, it may
corrupt the heap due to the use of MAP_FIXED to re-claim the stack
region.

Therefore, I propose to not unmap the stacks but to only remap them as
PROT_NONE in the !FLOATING_STACKS case, effectively reserving the
region.  This doesn't completely solve the problem but should be an
improvement.  Patch appended below.

I noticed that FLOATING_STACKS can only be enabled on i686.  Wouldn't
the ldt tricks work on i586, too?

Regards,
Wolfram.

2000-11-15  Wolfram Gloger  <wg@malloc.de>

	* manager.c (pthread_free()) [!FLOATING_STACKS]: Only remap the
	stack to PROT_NONE, don't unmap it, avoiding collisions with
	malloc.

--- manager.c	2000/11/14 17:30:27	1.1
+++ manager.c	2000/11/15 09:45:57
@@ -418,7 +418,7 @@
 
       new_thread_bottom = (char *) map_addr + guardsize;
       new_thread = ((pthread_descr) (new_thread_bottom + stacksize)) - 1;
-# else
+# else /* !FLOATING_STACKS */
       if (attr != NULL)
 	{
 	  guardsize = page_roundup (attr->__guardsize, granularity);
@@ -696,23 +696,24 @@
     {
       size_t guardsize = th->p_guardsize;
       /* Free the stack and thread descriptor area */
-#ifdef NEED_SEPARATE_REGISTER_STACK
       char *guardaddr = th->p_guardaddr;
-      /* We unmap exactly what we mapped, in case there was something
-	 else in the same region.  Guardaddr is always set, eve if
-	 guardsize is 0.  This allows us to compute everything else.  */
+      /* Guardaddr is always set, even if guardsize is 0.  This allows
+	 us to compute everything else.  */
       size_t stacksize = (char *)(th+1) - guardaddr - guardsize;
-      /* Unmap the register stack, which is below guardaddr.  */
-      munmap((caddr_t)(guardaddr-stacksize),
-	     2 * stacksize + th->p_guardsize);
+#ifdef NEED_SEPARATE_REGISTER_STACK
+      /* Take account of the register stack, which is below guardaddr.  */
+      guardaddr -= stacksize;
+      stacksize *= 2;
+#endif
+#if FLOATING_STACKS
+      /* Can unmap safely.  */
+      munmap(guardaddr, stacksize + guardsize);
 #else
-      char *guardaddr = th->p_guardaddr;
-      /* We unmap exactly what we mapped, in case there was something
-	 else in the same region.  Guardaddr is always set, eve if
-	 guardsize is 0.  This allows us to compute everything else.  */
-      size_t stacksize = (char *)(th+1) - guardaddr - guardsize;
-
-      munmap (guardaddr, stacksize + guardsize);
+      /* Only remap to PROT_NONE, so that the region is reserved in
+         case we map the stack again later.  Avoid collision with
+         other mmap()s, in particular by malloc().  */
+      mmap(guardaddr, stacksize + guardsize, PROT_NONE,
+	   MAP_PRIVATE | MAP_ANONYMOUS | MAP_FIXED, -1, 0);
 #endif
     }
 }