This is the mail archive of the libc-alpha@sourceware.org mailing list for the glibc project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

[PATCH] Fix bug of __register_atfork


Hi, all
	When I tested glibc, I found a bug of __register_atfork.

$ cat test.c
#include <stdio.h>
#include <stdlib.h>
//call glibc __register_atfork:
#include <pthread.h>
#define CALL__register_atfork pthread_atfork

int times = 20;
// usage argv[0] [times]

int main(int argc, char *argv[])
{
       int i;
       if (argc > 1) {
               times = atoi(argv[1]);
       }
	//malloc hasn't been called until here

	// call __register_atfork $times times totally.
	// $i = 1; because __register_atfork was called once
	// in __libc_pthread_init() before.
       for (i = 1; i < times; i++) {
               CALL__register_atfork(NULL, NULL, NULL);
       }

       malloc(128);
       return 0;
}

$ gcc test.c -o test -lpthread # Do not use --static, in static-linked program
                               # malloc may be called before main function and
                               # the bug will be not occurred.

# Test 1
$ ./test 48
Segmentation fault

# Test 2
$ ./test 49 # or greater
<the program doesn't exit, it sleeps on syscall futex forever>

	I read the code of glibc, and I found that __register_atfork and malloc
call each other. Glibc malloc calls __register_atfork when its init function
is called, and __register_atfork calls malloc when its pools are exhausted.
Especially pools are exhausted when __register_atfork is called at the 49th time.

	In the test 1, "malloc(128);" is the first time that malloc was called.
So malloc calls it's init function, and init function calls __register_atfork,
but it's the 49th time that __register_atfork is called, so __register_atfork
calls malloc. So malloc reenters itself and causes "Segmentation fault".

	In the test 2, when __register_atfork is called at the 49th time, it
calls malloc, and malloc calls __register_atfork. So __register_atfork  reenters
itself and sleeps on its low-level-lock.

	In a word, malloc and __register_atfork takes each other as its
foundation, so we have to uncouple them. In my patch, I use mmap instead of 
malloc as __register_atfork's foundation.


2007-10-25 Lai Jiangshan <laijs@cn.fujitsu.com>

	* nptl/sysdeps/unix/sysv/linux/register-atfork.c
	(struct fork_handler_pool) Rewrite
	(fork_handler_alloc) Use mmap to alloc memory
	(free_mem) Use munmap to free memory


--- glibc.orig/nptl/sysdeps/unix/sysv/linux/register-atfork.c	2005-12-22 06:17:21.000000000 +0800
+++ glibc.new/nptl/sysdeps/unix/sysv/linux/register-atfork.c	2007-10-23 21:27:12.000000000 +0800
@@ -20,52 +20,58 @@
 #include <errno.h>
 #include <stdlib.h>
 #include <string.h>
+#include <unistd.h>
 #include <fork.h>
+#include <sys/mman.h>
 
 
 /* Lock to protect allocation and deallocation of fork handlers.  */
 lll_lock_t __fork_lock = LLL_LOCK_INITIALIZER;
 
-
-/* Number of pre-allocated handler entries.  */
-#define NHANDLER 48
-
 /* Memory pool for fork handler structures.  */
 static struct fork_handler_pool
 {
   struct fork_handler_pool *next;
-  struct fork_handler mem[NHANDLER];
-} fork_handler_pool;
+  struct fork_handler mem[0];
+} *fork_handler_pool = NULL;
+static unsigned int fork_handler_pool_size = 0;
+static unsigned int nhandler = 0;
 
 
 static struct fork_handler *
 fork_handler_alloc (void)
 {
-  struct fork_handler_pool *runp = &fork_handler_pool;
+  struct fork_handler_pool *runp;
   struct fork_handler *result = NULL;
   unsigned int i;
 
-  do
+  /* Make sure fork_handler_pool_size and nhandler have been initialized */
+  if (fork_handler_pool_size == 0)
+    {
+      fork_handler_pool_size = __getpagesize();
+      nhandler = (fork_handler_pool_size - sizeof(struct fork_handler_pool))
+        / sizeof(struct fork_handler);
+    }
+
+  for (runp = fork_handler_pool; runp != NULL; runp = runp->next)
     {
       /* Search for an empty entry.  */
-      for (i = 0; i < NHANDLER; ++i)
-	if (runp->mem[i].refcntr == 0)
-	  goto found;
+      for (i = 0; i < nhandler; ++i)
+        if (runp->mem[i].refcntr == 0)
+          goto found;
     }
-  while ((runp = runp->next) != NULL);
 
   /* We have to allocate a new entry.  */
-  runp = (struct fork_handler_pool *) calloc (1, sizeof (*runp));
+  runp = (struct fork_handler_pool *) mmap (NULL, fork_handler_pool_size,
+    PROT_READ | PROT_WRITE, MAP_PRIVATE | MAP_ANONYMOUS, -1, 0);
+
   if (runp != NULL)
     {
       /* Enqueue the new memory pool into the list.  */
-      runp->next = fork_handler_pool.next;
-      fork_handler_pool.next = runp;
+      runp->next = fork_handler_pool;
+      fork_handler_pool = runp;
 
-      /* We use the last entry on the page.  This means when we start
-	 searching from the front the next time we will find the first
-	 entry unused.  */
-      i = NHANDLER - 1;
+      i = 0;
 
     found:
       result = &runp->mem[i];
@@ -118,9 +124,9 @@
   __fork_handlers = NULL;
 
   /* Free eventually alloated memory blocks for the object pool.  */
-  struct fork_handler_pool *runp = fork_handler_pool.next;
+  struct fork_handler_pool *runp = fork_handler_pool;
 
-  memset (&fork_handler_pool, '\0', sizeof (fork_handler_pool));
+  fork_handler_pool = NULL;
 
   /* Release the lock.  */
   lll_unlock (__fork_lock);
@@ -130,6 +136,6 @@
     {
       struct fork_handler_pool *oldp = runp;
       runp = runp->next;
-      free (oldp);
+      munmap (oldp, fork_handler_pool_size);
     }
 }


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]