[PATCH v3] Fix the race between atexit() and exit()

[Peng Haitao <penght at cn dot fujitsu dot 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 | 15 ++++++++---
 stdlib/exit.c       | 72 ++++++++++++++++++++++++++++++++++++++++++-----------
 stdlib/exit.h       |  6 +++++
 4 files changed, 91 insertions(+), 18 deletions(-)

diff --git a/ChangeLog b/ChangeLog
index 4f90243..171e05e 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,19 @@
+2013-03-01  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.
+
 2013-02-28  Joseph Myers  <joseph@codesourcery.com>
 
 	[BZ #13550]
diff --git a/stdlib/cxa_atexit.c b/stdlib/cxa_atexit.c
index 82e3365..2370c8a 100644
--- a/stdlib/cxa_atexit.c
+++ b/stdlib/cxa_atexit.c
@@ -1,4 +1,5 @@
 /* Copyright (C) 1999-2013 Free Software Foundation, Inc.
+   Copyright 2013 FUJITSU LIMITED
    This file is part of the GNU C Library.
 
    The GNU C Library is free software; you can redistribute it and/or
@@ -18,7 +19,6 @@
 #include <assert.h>
 #include <stdlib.h>
 
-#include <bits/libc-lock.h>
 #include "exit.h"
 #include <atomic.h>
 #include <sysdep.h>
@@ -60,7 +60,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 +75,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 +133,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 2e86caa..ec40a4d 100644
--- a/stdlib/exit.c
+++ b/stdlib/exit.c
@@ -1,4 +1,5 @@
 /* Copyright (C) 1991-2013 Free Software Foundation, Inc.
+   Copyright 2013 FUJITSU LIMITED
    This file is part of the GNU C Library.
 
    The GNU C Library is free software; you can redistribute it and/or
@@ -19,11 +20,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
@@ -40,25 +44,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
@@ -66,6 +89,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
@@ -73,20 +97,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 d96ed95..972da89 100644
--- a/stdlib/exit.h
+++ b/stdlib/exit.h
@@ -1,4 +1,5 @@
 /* Copyright (C) 1991-2013 Free Software Foundation, Inc.
+   Copyright 2013 FUJITSU LIMITED
    This file is part of the GNU C Library.
 
    The GNU C Library is free software; you can redistribute it and/or
@@ -20,6 +21,7 @@
 
 #include <stdbool.h>
 #include <stdint.h>
+#include <bits/libc-lock.h>
 
 enum
 {
@@ -72,5 +74,9 @@ extern int __internal_atexit (void (*func) (void *), void *arg, void *d,
   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.8.1.2