00db10
Backport of these upstream commits:
00db10
00db10
commit 29d794863cd6e03115d3670707cc873a9965ba92
00db10
Author: Florian Weimer <fweimer@redhat.com>
00db10
Date:   Thu Apr 14 09:17:02 2016 +0200
00db10
00db10
    malloc: Run fork handler as late as possible [BZ #19431]
00db10
    
00db10
    Previously, a thread M invoking fork would acquire locks in this order:
00db10
    
00db10
      (M1) malloc arena locks (in the registered fork handler)
00db10
      (M2) libio list lock
00db10
    
00db10
    A thread F invoking flush (NULL) would acquire locks in this order:
00db10
    
00db10
      (F1) libio list lock
00db10
      (F2) individual _IO_FILE locks
00db10
    
00db10
    A thread G running getdelim would use this order:
00db10
    
00db10
      (G1) _IO_FILE lock
00db10
      (G2) malloc arena lock
00db10
    
00db10
    After executing (M1), (F1), (G1), none of the threads can make progress.
00db10
    
00db10
    This commit changes the fork lock order to:
00db10
    
00db10
      (M'1) libio list lock
00db10
      (M'2) malloc arena locks
00db10
    
00db10
    It explicitly encodes the lock order in the implementations of fork,
00db10
    and does not rely on the registration order, thus avoiding the deadlock.
00db10
00db10
commit 186fe877f3df0b84d57dfbf0386f6332c6aa69bc
00db10
Author: Florian Weimer <fweimer@redhat.com>
00db10
Date:   Thu Apr 14 12:53:03 2016 +0200
00db10
00db10
    malloc: Add missing internal_function attributes on function definitions
00db10
    
00db10
    Fixes build on i386 after commit 29d794863cd6e03115d3670707cc873a9965ba92.
00db10
00db10
Index: b/malloc/Makefile
00db10
===================================================================
00db10
--- a/malloc/Makefile
00db10
+++ b/malloc/Makefile
00db10
@@ -28,7 +28,7 @@ tests := mallocbug tst-malloc tst-valloc
00db10
 	 tst-mallocstate tst-mcheck tst-mallocfork tst-trim1 \
00db10
 	 tst-malloc-usable \
00db10
 	 tst-malloc-backtrace tst-malloc-thread-exit \
00db10
-	 tst-malloc-thread-fail
00db10
+	 tst-malloc-thread-fail tst-malloc-fork-deadlock
00db10
 test-srcs = tst-mtrace
00db10
 
00db10
 routines = malloc morecore mcheck mtrace obstack
00db10
@@ -49,6 +49,7 @@ $(objpfx)tst-malloc-thread-fail: $(commo
00db10
 			       $(common-objpfx)nptl/libpthread_nonshared.a
00db10
 $(objpfx)tst-malloc-thread-exit: $(common-objpfx)nptl/libpthread.so \
00db10
 			       $(common-objpfx)nptl/libpthread_nonshared.a
00db10
+$(objpfx)tst-malloc-fork-deadlock: $(shared-thread-library)
00db10
 
00db10
 # These should be removed by `make clean'.
00db10
 extra-objs = mcheck-init.o libmcheck.a
00db10
Index: b/malloc/arena.c
00db10
===================================================================
00db10
--- a/malloc/arena.c
00db10
+++ b/malloc/arena.c
00db10
@@ -162,10 +162,6 @@ static void           (*save_free_hook)
00db10
 					 const __malloc_ptr_t);
00db10
 static void*        save_arena;
00db10
 
00db10
-#ifdef ATFORK_MEM
00db10
-ATFORK_MEM;
00db10
-#endif
00db10
-
00db10
 /* Magic value for the thread-specific arena pointer when
00db10
    malloc_atfork() is in use.  */
00db10
 
00db10
@@ -228,14 +224,15 @@ free_atfork(void* mem, const void *calle
00db10
 /* Counter for number of times the list is locked by the same thread.  */
00db10
 static unsigned int atfork_recursive_cntr;
00db10
 
00db10
-/* The following two functions are registered via thread_atfork() to
00db10
-   make sure that the mutexes remain in a consistent state in the
00db10
-   fork()ed version of a thread.  Also adapt the malloc and free hooks
00db10
-   temporarily, because the `atfork' handler mechanism may use
00db10
-   malloc/free internally (e.g. in LinuxThreads). */
00db10
+/* The following three functions are called around fork from a
00db10
+   multi-threaded process.  We do not use the general fork handler
00db10
+   mechanism to make sure that our handlers are the last ones being
00db10
+   called, so that other fork handlers can use the malloc
00db10
+   subsystem.  */
00db10
 
00db10
-static void
00db10
-ptmalloc_lock_all (void)
00db10
+void
00db10
+internal_function
00db10
+__malloc_fork_lock_parent (void)
00db10
 {
00db10
   mstate ar_ptr;
00db10
 
00db10
@@ -243,7 +240,7 @@ ptmalloc_lock_all (void)
00db10
     return;
00db10
 
00db10
   /* We do not acquire free_list_lock here because we completely
00db10
-     reconstruct free_list in ptmalloc_unlock_all2.  */
00db10
+     reconstruct free_list in __malloc_fork_unlock_child.  */
00db10
 
00db10
   if (mutex_trylock(&list_lock))
00db10
     {
00db10
@@ -268,7 +265,7 @@ ptmalloc_lock_all (void)
00db10
   __free_hook = free_atfork;
00db10
   /* Only the current thread may perform malloc/free calls now.
00db10
      save_arena will be reattached to the current thread, in
00db10
-     ptmalloc_lock_all, so save_arena->attached_threads is not
00db10
+     __malloc_fork_lock_parent, so save_arena->attached_threads is not
00db10
      updated.  */
00db10
   tsd_getspecific(arena_key, save_arena);
00db10
   tsd_setspecific(arena_key, ATFORK_ARENA_PTR);
00db10
@@ -276,8 +273,9 @@ ptmalloc_lock_all (void)
00db10
   ++atfork_recursive_cntr;
00db10
 }
00db10
 
00db10
-static void
00db10
-ptmalloc_unlock_all (void)
00db10
+void
00db10
+internal_function
00db10
+__malloc_fork_unlock_parent (void)
00db10
 {
00db10
   mstate ar_ptr;
00db10
 
00db10
@@ -286,8 +284,8 @@ ptmalloc_unlock_all (void)
00db10
   if (--atfork_recursive_cntr != 0)
00db10
     return;
00db10
   /* Replace ATFORK_ARENA_PTR with save_arena.
00db10
-     save_arena->attached_threads was not changed in ptmalloc_lock_all
00db10
-     and is still correct.  */
00db10
+     save_arena->attached_threads was not changed in
00db10
+     __malloc_fork_lock_parent and is still correct.  */
00db10
   tsd_setspecific(arena_key, save_arena);
00db10
   __malloc_hook = save_malloc_hook;
00db10
   __free_hook = save_free_hook;
00db10
@@ -299,15 +297,9 @@ ptmalloc_unlock_all (void)
00db10
   (void)mutex_unlock(&list_lock);
00db10
 }
00db10
 
00db10
-# ifdef __linux__
00db10
-
00db10
-/* In NPTL, unlocking a mutex in the child process after a
00db10
-   fork() is currently unsafe, whereas re-initializing it is safe and
00db10
-   does not leak resources.  Therefore, a special atfork handler is
00db10
-   installed for the child. */
00db10
-
00db10
-static void
00db10
-ptmalloc_unlock_all2 (void)
00db10
+void
00db10
+internal_function
00db10
+__malloc_fork_unlock_child (void)
00db10
 {
00db10
   mstate ar_ptr;
00db10
 
00db10
@@ -338,12 +330,6 @@ ptmalloc_unlock_all2 (void)
00db10
   atfork_recursive_cntr = 0;
00db10
 }
00db10
 
00db10
-# else
00db10
-
00db10
-#  define ptmalloc_unlock_all2 ptmalloc_unlock_all
00db10
-
00db10
-# endif
00db10
-
00db10
 #endif  /* !NO_THREADS */
00db10
 
00db10
 /* Initialization routine. */
00db10
@@ -413,7 +399,6 @@ ptmalloc_init (void)
00db10
 
00db10
   tsd_key_create(&arena_key, NULL);
00db10
   tsd_setspecific(arena_key, (void *)&main_arena);
00db10
-  thread_atfork(ptmalloc_lock_all, ptmalloc_unlock_all, ptmalloc_unlock_all2);
00db10
   const char *s = NULL;
00db10
   if (__builtin_expect (_environ != NULL, 1))
00db10
     {
00db10
@@ -487,12 +472,6 @@ ptmalloc_init (void)
00db10
   __malloc_initialized = 1;
00db10
 }
00db10
 
00db10
-/* There are platforms (e.g. Hurd) with a link-time hook mechanism. */
00db10
-#ifdef thread_atfork_static
00db10
-thread_atfork_static(ptmalloc_lock_all, ptmalloc_unlock_all, \
00db10
-		     ptmalloc_unlock_all2)
00db10
-#endif
00db10
-
00db10
 
00db10
 
00db10
 /* Managing heaps and arenas (for concurrent threads) */
00db10
@@ -827,7 +806,8 @@ _int_new_arena(size_t size)
00db10
      limit is reached).  At this point, some arena has to be attached
00db10
      to two threads.  We could acquire the arena lock before list_lock
00db10
      to make it less likely that reused_arena picks this new arena,
00db10
-     but this could result in a deadlock with ptmalloc_lock_all.  */
00db10
+     but this could result in a deadlock with
00db10
+     __malloc_fork_lock_parent.  */
00db10
 
00db10
   (void) mutex_lock (&a->mutex);
00db10
 
00db10
Index: b/malloc/malloc-internal.h
00db10
===================================================================
00db10
--- /dev/null
00db10
+++ b/malloc/malloc-internal.h
00db10
@@ -0,0 +1,32 @@
00db10
+/* Internal declarations for malloc, for use within libc.
00db10
+   Copyright (C) 2016 Free Software Foundation, Inc.
00db10
+   This file is part of the GNU C Library.
00db10
+
00db10
+   The GNU C Library is free software; you can redistribute it and/or
00db10
+   modify it under the terms of the GNU Lesser General Public License as
00db10
+   published by the Free Software Foundation; either version 2.1 of the
00db10
+   License, or (at your option) any later version.
00db10
+
00db10
+   The GNU C Library is distributed in the hope that it will be useful,
00db10
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
00db10
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
00db10
+   Lesser General Public License for more details.
00db10
+
00db10
+   You should have received a copy of the GNU Lesser General Public
00db10
+   License along with the GNU C Library; see the file COPYING.LIB.  If
00db10
+   not, see <http://www.gnu.org/licenses/>.  */
00db10
+
00db10
+#ifndef _MALLOC_PRIVATE_H
00db10
+#define _MALLOC_PRIVATE_H
00db10
+
00db10
+/* Called in the parent process before a fork.  */
00db10
+void __malloc_fork_lock_parent (void) internal_function attribute_hidden;
00db10
+
00db10
+/* Called in the parent process after a fork.  */
00db10
+void __malloc_fork_unlock_parent (void) internal_function attribute_hidden;
00db10
+
00db10
+/* Called in the child process after a fork.  */
00db10
+void __malloc_fork_unlock_child (void) internal_function attribute_hidden;
00db10
+
00db10
+
00db10
+#endif /* _MALLOC_PRIVATE_H */
00db10
Index: b/malloc/malloc.c
00db10
===================================================================
00db10
--- a/malloc/malloc.c
00db10
+++ b/malloc/malloc.c
00db10
@@ -291,6 +291,7 @@ __malloc_assert (const char *assertion,
00db10
 }
00db10
 #endif
00db10
 
00db10
+#include <malloc/malloc-internal.h>
00db10
 
00db10
 /*
00db10
   INTERNAL_SIZE_T is the word-size used for internal bookkeeping
00db10
Index: b/malloc/tst-malloc-fork-deadlock.c
00db10
===================================================================
00db10
--- /dev/null
00db10
+++ b/malloc/tst-malloc-fork-deadlock.c
00db10
@@ -0,0 +1,220 @@
00db10
+/* Test concurrent fork, getline, and fflush (NULL).
00db10
+   Copyright (C) 2016 Free Software Foundation, Inc.
00db10
+   This file is part of the GNU C Library.
00db10
+
00db10
+   The GNU C Library is free software; you can redistribute it and/or
00db10
+   modify it under the terms of the GNU Lesser General Public License as
00db10
+   published by the Free Software Foundation; either version 2.1 of the
00db10
+   License, or (at your option) any later version.
00db10
+
00db10
+   The GNU C Library is distributed in the hope that it will be useful,
00db10
+   but WITHOUT ANY WARRANTY; without even the implied warranty of
00db10
+   MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
00db10
+   Lesser General Public License for more details.
00db10
+
00db10
+   You should have received a copy of the GNU Lesser General Public
00db10
+   License along with the GNU C Library; see the file COPYING.LIB.  If
00db10
+   not, see <http://www.gnu.org/licenses/>.  */
00db10
+
00db10
+#include <sys/wait.h>
00db10
+#include <unistd.h>
00db10
+#include <errno.h>
00db10
+#include <stdio.h>
00db10
+#include <pthread.h>
00db10
+#include <stdbool.h>
00db10
+#include <stdlib.h>
00db10
+#include <malloc.h>
00db10
+#include <time.h>
00db10
+#include <string.h>
00db10
+#include <signal.h>
00db10
+
00db10
+static int do_test (void);
00db10
+#define TEST_FUNCTION do_test ()
00db10
+#include "../test-skeleton.c"
00db10
+
00db10
+enum {
00db10
+  /* Number of threads which call fork.  */
00db10
+  fork_thread_count = 4,
00db10
+  /* Number of threads which call getline (and, indirectly,
00db10
+     malloc).  */
00db10
+  read_thread_count = 8,
00db10
+};
00db10
+
00db10
+static bool termination_requested;
00db10
+
00db10
+static void *
00db10
+fork_thread_function (void *closure)
00db10
+{
00db10
+  while (!__atomic_load_n (&termination_requested, __ATOMIC_RELAXED))
00db10
+    {
00db10
+      pid_t pid = fork ();
00db10
+      if (pid < 0)
00db10
+        {
00db10
+          printf ("error: fork: %m\n");
00db10
+          abort ();
00db10
+        }
00db10
+      else if (pid == 0)
00db10
+        _exit (17);
00db10
+
00db10
+      int status;
00db10
+      if (waitpid (pid, &status, 0) < 0)
00db10
+        {
00db10
+          printf ("error: waitpid: %m\n");
00db10
+          abort ();
00db10
+        }
00db10
+      if (!WIFEXITED (status) || WEXITSTATUS (status) != 17)
00db10
+        {
00db10
+          printf ("error: waitpid returned invalid status: %d\n", status);
00db10
+          abort ();
00db10
+        }
00db10
+    }
00db10
+  return NULL;
00db10
+}
00db10
+
00db10
+static char *file_to_read;
00db10
+
00db10
+static void *
00db10
+read_thread_function (void *closure)
00db10
+{
00db10
+  FILE *f = fopen (file_to_read, "r");
00db10
+  if (f == NULL)
00db10
+    {
00db10
+      printf ("error: fopen (%s): %m\n", file_to_read);
00db10
+      abort ();
00db10
+    }
00db10
+
00db10
+  while (!__atomic_load_n (&termination_requested, __ATOMIC_RELAXED))
00db10
+    {
00db10
+      rewind (f);
00db10
+      char *line = NULL;
00db10
+      size_t line_allocated = 0;
00db10
+      ssize_t ret = getline (&line, &line_allocated, f);
00db10
+      if (ret < 0)
00db10
+        {
00db10
+          printf ("error: getline: %m\n");
00db10
+          abort ();
00db10
+        }
00db10
+      free (line);
00db10
+    }
00db10
+  fclose (f);
00db10
+
00db10
+  return NULL;
00db10
+}
00db10
+
00db10
+static void *
00db10
+flushall_thread_function (void *closure)
00db10
+{
00db10
+  while (!__atomic_load_n (&termination_requested, __ATOMIC_RELAXED))
00db10
+    if (fflush (NULL) != 0)
00db10
+      {
00db10
+        printf ("error: fflush (NULL): %m\n");
00db10
+        abort ();
00db10
+      }
00db10
+  return NULL;
00db10
+}
00db10
+
00db10
+static void
00db10
+create_threads (pthread_t *threads, size_t count, void *(*func) (void *))
00db10
+{
00db10
+  for (size_t i = 0; i < count; ++i)
00db10
+    {
00db10
+      int ret = pthread_create (threads + i, NULL, func, NULL);
00db10
+      if (ret != 0)
00db10
+        {
00db10
+          errno = ret;
00db10
+          printf ("error: pthread_create: %m\n");
00db10
+          abort ();
00db10
+        }
00db10
+    }
00db10
+}
00db10
+
00db10
+static void
00db10
+join_threads (pthread_t *threads, size_t count)
00db10
+{
00db10
+  for (size_t i = 0; i < count; ++i)
00db10
+    {
00db10
+      int ret = pthread_join (threads[i], NULL);
00db10
+      if (ret != 0)
00db10
+        {
00db10
+          errno = ret;
00db10
+          printf ("error: pthread_join: %m\n");
00db10
+          abort ();
00db10
+        }
00db10
+    }
00db10
+}
00db10
+
00db10
+/* Create a file which consists of a single long line, and assigns
00db10
+   file_to_read.  The hope is that this triggers an allocation in
00db10
+   getline which needs a lock.  */
00db10
+static void
00db10
+create_file_with_large_line (void)
00db10
+{
00db10
+  int fd = create_temp_file ("bug19431-large-line", &file_to_read);
00db10
+  if (fd < 0)
00db10
+    {
00db10
+      printf ("error: create_temp_file: %m\n");
00db10
+      abort ();
00db10
+    }
00db10
+  FILE *f = fdopen (fd, "w+");
00db10
+  if (f == NULL)
00db10
+    {
00db10
+      printf ("error: fdopen: %m\n");
00db10
+      abort ();
00db10
+    }
00db10
+  for (int i = 0; i < 50000; ++i)
00db10
+    fputc ('x', f);
00db10
+  fputc ('\n', f);
00db10
+  if (ferror (f))
00db10
+    {
00db10
+      printf ("error: fputc: %m\n");
00db10
+      abort ();
00db10
+    }
00db10
+  if (fclose (f) != 0)
00db10
+    {
00db10
+      printf ("error: fclose: %m\n");
00db10
+      abort ();
00db10
+    }
00db10
+}
00db10
+
00db10
+static int
00db10
+do_test (void)
00db10
+{
00db10
+  /* Make sure that we do not exceed the arena limit with the number
00db10
+     of threads we configured.  */
00db10
+  if (mallopt (M_ARENA_MAX, 400) == 0)
00db10
+    {
00db10
+      printf ("error: mallopt (M_ARENA_MAX) failed\n");
00db10
+      return 1;
00db10
+    }
00db10
+
00db10
+  /* Leave some room for shutting down all threads gracefully.  */
00db10
+  int timeout = 3;
00db10
+  if (timeout > TIMEOUT)
00db10
+    timeout = TIMEOUT - 1;
00db10
+
00db10
+  create_file_with_large_line ();
00db10
+
00db10
+  pthread_t fork_threads[fork_thread_count];
00db10
+  create_threads (fork_threads, fork_thread_count, fork_thread_function);
00db10
+  pthread_t read_threads[read_thread_count];
00db10
+  create_threads (read_threads, read_thread_count, read_thread_function);
00db10
+  pthread_t flushall_threads[1];
00db10
+  create_threads (flushall_threads, 1, flushall_thread_function);
00db10
+
00db10
+  struct timespec ts = {timeout, 0};
00db10
+  if (nanosleep (&ts, NULL))
00db10
+    {
00db10
+      printf ("error: error: nanosleep: %m\n");
00db10
+      abort ();
00db10
+    }
00db10
+
00db10
+  __atomic_store_n (&termination_requested, true, __ATOMIC_RELAXED);
00db10
+
00db10
+  join_threads (flushall_threads, 1);
00db10
+  join_threads (read_threads, read_thread_count);
00db10
+  join_threads (fork_threads, fork_thread_count);
00db10
+
00db10
+  free (file_to_read);
00db10
+
00db10
+  return 0;
00db10
+}
00db10
Index: b/manual/memory.texi
00db10
===================================================================
00db10
--- a/manual/memory.texi
00db10
+++ b/manual/memory.texi
00db10
@@ -1055,14 +1055,6 @@ systems that do not support @w{ISO C11}.
00db10
 @c     _dl_addr_inside_object ok
00db10
 @c    determine_info ok
00db10
 @c    __rtld_lock_unlock_recursive (dl_load_lock) @aculock
00db10
-@c   thread_atfork @asulock @aculock @acsfd @acsmem
00db10
-@c    __register_atfork @asulock @aculock @acsfd @acsmem
00db10
-@c     lll_lock (__fork_lock) @asulock @aculock
00db10
-@c     fork_handler_alloc @asulock @aculock @acsfd @acsmem
00db10
-@c      calloc dup @asulock @aculock @acsfd @acsmem
00db10
-@c     __linkin_atfork ok
00db10
-@c      catomic_compare_and_exchange_bool_acq ok
00db10
-@c     lll_unlock (__fork_lock) @aculock
00db10
 @c   *_environ @mtsenv
00db10
 @c   next_env_entry ok
00db10
 @c   strcspn dup ok
00db10
Index: b/nptl/sysdeps/unix/sysv/linux/fork.c
00db10
===================================================================
00db10
--- a/nptl/sysdeps/unix/sysv/linux/fork.c
00db10
+++ b/nptl/sysdeps/unix/sysv/linux/fork.c
00db10
@@ -29,7 +29,7 @@
00db10
 #include <bits/stdio-lock.h>
00db10
 #include <atomic.h>
00db10
 #include <pthreadP.h>
00db10
-
00db10
+#include <malloc/malloc-internal.h>
00db10
 
00db10
 unsigned long int *__fork_generation_pointer;
00db10
 
00db10
@@ -116,6 +116,11 @@ __libc_fork (void)
00db10
 
00db10
   _IO_list_lock ();
00db10
 
00db10
+  /* Acquire malloc locks.  This needs to come last because fork
00db10
+     handlers may use malloc, and the libio list lock has an indirect
00db10
+     malloc dependency as well (via the getdelim function).  */
00db10
+  __malloc_fork_lock_parent ();
00db10
+
00db10
 #ifndef NDEBUG
00db10
   pid_t ppid = THREAD_GETMEM (THREAD_SELF, tid);
00db10
 #endif
00db10
@@ -172,6 +177,9 @@ __libc_fork (void)
00db10
 # endif
00db10
 #endif
00db10
 
00db10
+      /* Release malloc locks.  */
00db10
+      __malloc_fork_unlock_child ();
00db10
+
00db10
       /* Reset the file list.  These are recursive mutexes.  */
00db10
       fresetlockfiles ();
00db10
 
00db10
@@ -213,6 +221,9 @@ __libc_fork (void)
00db10
       /* Restore the PID value.  */
00db10
       THREAD_SETMEM (THREAD_SELF, pid, parentpid);
00db10
 
00db10
+      /* Release malloc locks, parent process variant.  */
00db10
+      __malloc_fork_unlock_parent ();
00db10
+
00db10
       /* We execute this even if the 'fork' call failed.  */
00db10
       _IO_list_unlock ();
00db10