This is the mail archive of the
libc-alpha@sourceware.org
mailing list for the glibc project.
[PATCH v2] Fix the race between atexit() and exit()
- From: Peng Haitao <penght at cn dot fujitsu dot com>
- To: roland at hack dot frob dot com
- Cc: carlos at systemhalted dot org, drepper at gmail dot com, libc-alpha at sourceware dot org
- Date: Tue, 10 Jul 2012 10:51:09 +0800
- Subject: [PATCH v2] Fix the race between atexit() and exit()
- References: <20120709160210.5BC5E2C0B2@topped-with-meat.com>
exit() uses global variable __exit_funcs indirectly, which are not protected.
It is not safe in multithread circumstance.
When call exit() and atexit() simultaneously in multithread circumstance,
the following case will cause unsafe.
The case has main process A and thread B.
a. thread B call atexit()
b. process A call exit() to traverse the __exit_funcs list
c. thread B call calloc() to create a new entry p, and next to listp:
p->next = *listp;
d. process A modify listp to cur's next, then free cur:
*listp = cur->next;
e. thread B modify listp to p:
*listp = p;
f. when get f, the f is undefined:
const struct exit_function *const f =
&cur->fns[--cur->idx];
g. programme may be Segmentation fault
Signed-off-by: Peng Haitao <penght@cn.fujitsu.com>
---
ChangeLog | 16 +++++++++++
stdlib/cxa_atexit.c | 16 +++++++----
stdlib/exit.c | 74 ++++++++++++++++++++++++++++++++++++++++-----------
stdlib/exit.h | 11 +++++---
4 files changed, 93 insertions(+), 24 deletions(-)
diff --git a/ChangeLog b/ChangeLog
index c7070c5..613ce75 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,19 @@
+2012-07-10 Peng Haitao <penght@cn.fujitsu.com>
+
+ [BZ #14333]
+ * stdlib/cxa_atexit.c: Do not include bits/libc-lock.h.
+ (__libc_lock_define_initialized): Parameter 'lock' changed to global
+ lock '__exit_funcs_lock'.
+ (__new_exitfn): Fail new registration if exit code finished processing
+ all handlers.
+ * stdlib/exit.c: Include atomic.h.
+ (__exit_funcs_done): New flag to indicate status of list traversal.
+ (exit): Changes the handler processing style to that of
+ __cxa_finalize, with locking.
+ * stdlib/exit.h: Include bits/libc-lock.h.
+ (__exit_funcs_lock): Declaration for external access.
+ (__exit_funcs_done): Declaration for external access.
+
2012-07-09 Roland McGrath <roland@hack.frob.com>
[BZ #14336]
diff --git a/stdlib/cxa_atexit.c b/stdlib/cxa_atexit.c
index 9704398..24f110e 100644
--- a/stdlib/cxa_atexit.c
+++ b/stdlib/cxa_atexit.c
@@ -1,4 +1,4 @@
-/* Copyright (C) 1999,2001,2002,2005,2006,2009 Free Software Foundation, Inc.
+/* Copyright (C) 1999-2012 Free Software Foundation, Inc.
This file is part of the GNU C Library.
The GNU C Library is free software; you can redistribute it and/or
@@ -18,7 +18,6 @@
#include <assert.h>
#include <stdlib.h>
-#include <bits/libc-lock.h>
#include "exit.h"
#include <atomic.h>
#include <sysdep.h>
@@ -60,7 +59,7 @@ INTDEF(__cxa_atexit)
/* We change global data, so we need locking. */
-__libc_lock_define_initialized (static, lock)
+__libc_lock_define_initialized (, __exit_funcs_lock)
static struct exit_function_list initial;
@@ -75,7 +74,14 @@ __new_exitfn (struct exit_function_list **listp)
struct exit_function *r = NULL;
size_t i = 0;
- __libc_lock_lock (lock);
+ __libc_lock_lock (__exit_funcs_lock);
+
+ if (__exit_funcs_done)
+ {
+ /* exit code finished processing all handlers, so fail registration. */
+ __libc_lock_unlock (__exit_funcs_lock);
+ return NULL;
+ }
for (l = *listp; l != NULL; p = l, l = l->next)
{
@@ -126,7 +132,7 @@ __new_exitfn (struct exit_function_list **listp)
++__new_exitfn_called;
}
- __libc_lock_unlock (lock);
+ __libc_lock_unlock (__exit_funcs_lock);
return r;
}
diff --git a/stdlib/exit.c b/stdlib/exit.c
index 1ad548f..d686de5 100644
--- a/stdlib/exit.c
+++ b/stdlib/exit.c
@@ -1,5 +1,4 @@
-/* Copyright (C) 1991,95,96,97,99,2001,2002,2005,2009
- Free Software Foundation, Inc.
+/* Copyright (C) 1991-2012 Free Software Foundation, Inc.
This file is part of the GNU C Library.
The GNU C Library is free software; you can redistribute it and/or
@@ -20,11 +19,14 @@
#include <stdlib.h>
#include <unistd.h>
#include <sysdep.h>
+#include <atomic.h>
#include "exit.h"
#include "set-hooks.h"
DEFINE_HOOK (__libc_atexit, (void))
+/* initialize the processing complete flag to false. */
+bool __exit_funcs_done = false;
/* Call all functions registered with `atexit' and `on_exit',
in the reverse of the order in which they were registered
@@ -38,25 +40,44 @@ __run_exit_handlers (int status, struct exit_function_list **listp,
the functions registered with `atexit' and `on_exit'. We call
everyone on the list and use the status value in the last
exit (). */
- while (*listp != NULL)
+ while (1)
{
- struct exit_function_list *cur = *listp;
+ struct exit_function_list *cur;
+ struct exit_function *f;
- while (cur->idx > 0)
- {
- const struct exit_function *const f =
- &cur->fns[--cur->idx];
- switch (f->flavor)
+restart:
+ __libc_lock_lock (__exit_funcs_lock);
+
+ cur = *listp;
+ if (cur == NULL)
+ {
+ /* mark the processing complete,
+ we won't allow to register more handlers. */
+ __exit_funcs_done = true;
+ __libc_lock_unlock (__exit_funcs_lock);
+ break;
+ }
+
+ f = &cur->fns[cur->idx];
+ uint64_t check = __new_exitfn_called;
+
+ __libc_lock_unlock (__exit_funcs_lock);
+
+ while (f > &cur->fns[0])
+ {
+ switch ((--f)->flavor)
{
void (*atfct) (void);
void (*onfct) (int status, void *arg);
void (*cxafct) (void *arg, int status);
case ef_free:
+ break;
case ef_us:
- break;
+ goto restart;
case ef_on:
onfct = f->func.on.fn;
+ atomic_compare_and_exchange_bool_acq (&f->flavor, ef_free, ef_on);
#ifdef PTR_DEMANGLE
PTR_DEMANGLE (onfct);
#endif
@@ -64,6 +85,7 @@ __run_exit_handlers (int status, struct exit_function_list **listp,
break;
case ef_at:
atfct = f->func.at;
+ atomic_compare_and_exchange_bool_acq (&f->flavor, ef_free, ef_at);
#ifdef PTR_DEMANGLE
PTR_DEMANGLE (atfct);
#endif
@@ -71,20 +93,40 @@ __run_exit_handlers (int status, struct exit_function_list **listp,
break;
case ef_cxa:
cxafct = f->func.cxa.fn;
+ atomic_compare_and_exchange_bool_acq (&f->flavor, ef_free, ef_cxa);
#ifdef PTR_DEMANGLE
PTR_DEMANGLE (cxafct);
#endif
cxafct (f->func.cxa.arg, status);
break;
}
+ /* It is possible that last exit function registered
+ more exit functions. Start the loop over. */
+ __libc_lock_lock (__exit_funcs_lock);
+ if (__builtin_expect (check != __new_exitfn_called, 0))
+ {
+ __libc_lock_unlock (__exit_funcs_lock);
+ goto restart;
+ }
+ __libc_lock_unlock (__exit_funcs_lock);
}
- *listp = cur->next;
- if (*listp != NULL)
- /* Don't free the last element in the chain, this is the statically
- allocate element. */
- free (cur);
- }
+ __libc_lock_lock (__exit_funcs_lock);
+ if (__builtin_expect (check != __new_exitfn_called, 0))
+ {
+ __libc_lock_unlock (__exit_funcs_lock);
+ goto restart;
+ }
+ else
+ {
+ *listp = cur->next;
+ if (*listp != NULL)
+ /* Don't free the last element in the chain, this is the statically
+ allocate element. */
+ free (cur);
+ __libc_lock_unlock (__exit_funcs_lock);
+ }
+ }
if (run_list_atexit)
RUN_HOOK (__libc_atexit, ());
diff --git a/stdlib/exit.h b/stdlib/exit.h
index 818c7ac..25a398e 100644
--- a/stdlib/exit.h
+++ b/stdlib/exit.h
@@ -21,6 +21,7 @@
#include <stdbool.h>
#include <stdint.h>
+#include <bits/libc-lock.h>
enum
{
@@ -66,12 +67,16 @@ extern uint64_t __new_exitfn_called attribute_hidden;
extern void __run_exit_handlers (int status, struct exit_function_list **listp,
bool run_list_atexit)
- attribute_hidden __attribute__ ((__noreturn__));
+ attribute_hidden __attribute__ ((__noreturn__));
extern int __internal_atexit (void (*func) (void *), void *arg, void *d,
- struct exit_function_list **listp)
- attribute_hidden;
+ struct exit_function_list **listp)
+ attribute_hidden;
extern int __cxa_at_quick_exit (void (*func) (void *), void *d);
+__libc_lock_define (extern, __exit_funcs_lock);
+
+/* flag to mark that all registered atexit/on_exit handlers are called. */
+extern bool __exit_funcs_done;
#endif /* exit.h */
--
1.7.10.4