This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
[PATCH] Fix bug of __register_atfork
- From: Lai Jiangshan <laijs at cn dot fujitsu dot com>
- To: libc-alpha at sourceware dot org
- Date: Thu, 25 Oct 2007 16:56:30 +0800
- Subject: [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);
}
}