bug-glibc
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: mmap(...MAP_FIXED...) in pthread_allocate_stack ???


From: Wolfram Gloger
Subject: Re: mmap(...MAP_FIXED...) in pthread_allocate_stack ???
Date: Fri, 8 Dec 2000 09:51:10 +0100 ("MET)

Hello,

> the routine pthread_allocate_stack (linuxthreads/manager.c)
> contains this piece of code:
> 
>       map_addr = mmap((caddr_t)((char *)(new_thread + 1)
>                       - stacksize - guardsize),
>                 stacksize + guardsize,
>                 PROT_READ | PROT_WRITE | PROT_EXEC,
>                 MAP_PRIVATE | MAP_ANONYMOUS | MAP_FIXED, -1, 0);
>       if (map_addr == MAP_FAILED)
>         /* Bad luck, this segment is already mapped. */
>         return -1;
> 
> This would appear to be broken:  when you call mmap with
> MAP_FIXED, the kernel will simply *replace* any pre-existing
> mappings in this range.   From the comment it appears that
> the author expected the kernel to return MAP_FAILED in this case,
> which is in fact not true.

I agree that the comment is broken, however the MAP_FIXED is more or
less unavoidable with the current pthread design.  Exception: if there
is a register available to hold the thread_self pointer, then
FLOATING_STACKS can be defined in this file and MAP_FIXED isn't used.
Check out i686/useldt.h for an example.

> The effect is that if you create too many threads, your shared
> libraries or other mmap()ed areas will simply be overwritten
> with thread stacks ...

Yes, I noticed that on i586 as well, wenn the address space becomes
close to full.  I've created a patch to alleviate that problem
(although admittedly it is no complete solution), which I append
below.  Basically, if you create a number N of threads early on in the
program, and don't exceed that number later, everything is fine
(whereas without the patch, every thread that exits and is created
again causes one dangerous MAP_FIXED call).

Regards,
Wolfram.

2000-11-15  Wolfram Gloger  <address@hidden>

        * 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
     }
 }



reply via email to

[Prev in Thread] Current Thread [Next in Thread]