diff options
| -rw-r--r-- | ChangeLog | 10 | ||||
| -rw-r--r-- | malloc/Makefile | 3 | ||||
| -rw-r--r-- | malloc/tst-mallocfork2.c | 212 | ||||
| -rw-r--r-- | sysdeps/nptl/fork.c | 53 |
4 files changed, 262 insertions, 16 deletions
diff --git a/ChangeLog b/ChangeLog index dee85ca13e..e623247085 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,15 @@ 2016-05-12 Florian Weimer <fweimer@redhat.com> + [BZ #19703] + Partially async-signal-safe fork for single-threaded processes. + * sysdeps/nptl/fork.c (__libc_fork): Introduce multiple_threads + variable. Do not acquire and reset/release malloc and libio locks + in single-threaded processes. + * malloc/tst-mallocfork2.c: New file. + * malloc/Makefile (tests): Add it. + +2016-05-12 Florian Weimer <fweimer@redhat.com> + * sysdeps/posix/getaddrinfo.c (gaih_inet_serv): Add tmpbuf argument. Use scratch buffer instead of extend_alloca. (gethosts): Use scratch buffer instead of extend_alloca. diff --git a/malloc/Makefile b/malloc/Makefile index 3283f4f75f..fa1730ecb7 100644 --- a/malloc/Makefile +++ b/malloc/Makefile @@ -29,7 +29,8 @@ tests := mallocbug tst-malloc tst-valloc tst-calloc tst-obstack \ tst-malloc-usable tst-realloc tst-posix_memalign \ tst-pvalloc tst-memalign tst-mallopt tst-scratch_buffer \ tst-malloc-backtrace tst-malloc-thread-exit \ - tst-malloc-thread-fail tst-malloc-fork-deadlock + tst-malloc-thread-fail tst-malloc-fork-deadlock \ + tst-mallocfork2 test-srcs = tst-mtrace routines = malloc morecore mcheck mtrace obstack \ diff --git a/malloc/tst-mallocfork2.c b/malloc/tst-mallocfork2.c new file mode 100644 index 0000000000..a9e3e94aad --- /dev/null +++ b/malloc/tst-mallocfork2.c @@ -0,0 +1,212 @@ +/* Test case for async-signal-safe fork (with respect to malloc). + Copyright (C) 2016 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 + modify it under the terms of the GNU Lesser General Public License as + published by the Free Software Foundation; either version 2.1 of the + License, or (at your option) any later version. + + The GNU C Library is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + Lesser General Public License for more details. + + You should have received a copy of the GNU Lesser General Public + License along with the GNU C Library; see the file COPYING.LIB. If + not, see <http://www.gnu.org/licenses/>. */ + +/* This test will fail if the process is multi-threaded because we + only have an async-signal-safe fork in the single-threaded case + (where we skip acquiring the malloc heap locks). + + This test only checks async-signal-safety with regards to malloc; + other, more rarely-used glibc subsystems could have locks which + still make fork unsafe, even in single-threaded processes. */ + +#include <errno.h> +#include <signal.h> +#include <stdbool.h> +#include <stdio.h> +#include <stdlib.h> +#include <string.h> +#include <sys/wait.h> +#include <time.h> +#include <unistd.h> + +/* How many malloc objects to keep arond. */ +enum { malloc_objects = 1009 }; + +/* The maximum size of an object. */ +enum { malloc_maximum_size = 70000 }; + +/* How many signals need to be delivered before the test exits. */ +enum { signal_count = 1000 }; + + +/* Process ID of the subprocess which sends SIGUSR1 signals. */ +static pid_t sigusr1_sender_pid; + +/* Set to 1 if SIGUSR1 is received. Used to detect a signal during + malloc/free. */ +static volatile sig_atomic_t sigusr1_received; + +/* Periodically set to 1, to indicate that the process is making + progress. Checked by liveness_signal_handler. */ +static volatile sig_atomic_t progress_indicator = 1; + +/* Write the message to standard output. Usable from signal + handlers. */ +static void +write_message (const char *str) +{ + write (STDOUT_FILENO, str, strlen (str)); +} + +static void +sigusr1_handler (int signo) +{ + /* Let the main program make progress, by temporarily suspending + signals from the subprocess. */ + if (sigusr1_received) + return; + if (kill (sigusr1_sender_pid, SIGSTOP) != 0) + { + write_message ("error: kill (SIGSTOP)\n"); + abort (); + } + sigusr1_received = 1; + + /* Perform a fork with a trivial subprocess. */ + pid_t pid = fork (); + if (pid == -1) + { + write_message ("error: fork\n"); + abort (); + } + if (pid == 0) + _exit (0); + int status; + int ret = TEMP_FAILURE_RETRY (waitpid (pid, &status, 0)); + if (ret < 0) + { + write_message ("error: waitpid\n"); + abort (); + } + if (status != 0) + { + write_message ("error: unexpected exit status from subprocess\n"); + abort (); + } +} + +static void +liveness_signal_handler (int signo) +{ + if (progress_indicator) + progress_indicator = 0; + else + write_message ("warning: process seems to be stuck\n"); +} + +static void +__attribute__ ((noreturn)) +signal_sender (int signo, bool sleep) +{ + pid_t target = getppid (); + while (true) + { + if (kill (target, signo) != 0) + { + dprintf (STDOUT_FILENO, "error: kill: %m\n"); + abort (); + } + if (sleep) + usleep (1 * 1000 * 1000); + } +} + +static int +do_test (void) +{ + struct sigaction action = + { + .sa_handler = sigusr1_handler, + }; + sigemptyset (&action.sa_mask); + + if (sigaction (SIGUSR1, &action, NULL) != 0) + { + printf ("error: sigaction: %m"); + return 1; + } + + action.sa_handler = liveness_signal_handler; + if (sigaction (SIGUSR2, &action, NULL) != 0) + { + printf ("error: sigaction: %m"); + return 1; + } + + pid_t sigusr2_sender_pid = fork (); + if (sigusr2_sender_pid == 0) + signal_sender (SIGUSR2, true); + sigusr1_sender_pid = fork (); + if (sigusr1_sender_pid == 0) + signal_sender (SIGUSR1, false); + + void *objects[malloc_objects] = {}; + unsigned signals = 0; + unsigned seed = 1; + time_t last_report = 0; + while (signals < signal_count) + { + progress_indicator = 1; + int slot = rand_r (&seed) % malloc_objects; + size_t size = rand_r (&seed) % malloc_maximum_size; + if (kill (sigusr1_sender_pid, SIGCONT) != 0) + { + printf ("error: kill (SIGCONT): %m\n"); + signal (SIGUSR1, SIG_IGN); + kill (sigusr1_sender_pid, SIGKILL); + kill (sigusr2_sender_pid, SIGKILL); + return 1; + } + sigusr1_received = false; + free (objects[slot]); + objects[slot] = malloc (size); + if (sigusr1_received) + { + ++signals; + time_t current = time (0); + if (current != last_report) + { + printf ("info: SIGUSR1 signal count: %u\n", signals); + last_report = current; + } + } + if (objects[slot] == NULL) + { + printf ("error: malloc: %m\n"); + signal (SIGUSR1, SIG_IGN); + kill (sigusr1_sender_pid, SIGKILL); + kill (sigusr2_sender_pid, SIGKILL); + return 1; + } + } + + /* Clean up allocations. */ + for (int slot = 0; slot < malloc_objects; ++slot) + free (objects[slot]); + + /* Terminate the signal-sending subprocess. The SIGUSR1 handler + should no longer run because it uses sigusr1_sender_pid. */ + signal (SIGUSR1, SIG_IGN); + kill (sigusr1_sender_pid, SIGKILL); + kill (sigusr2_sender_pid, SIGKILL); + + return 0; +} + +#define TEST_FUNCTION do_test () +#include "../test-skeleton.c" diff --git a/sysdeps/nptl/fork.c b/sysdeps/nptl/fork.c index 1a68cbd647..616d897a36 100644 --- a/sysdeps/nptl/fork.c +++ b/sysdeps/nptl/fork.c @@ -54,6 +54,12 @@ __libc_fork (void) struct used_handler *next; } *allp = NULL; + /* Determine if we are running multiple threads. We skip some fork + handlers in the single-thread case, to make fork safer to use in + signal handlers. POSIX requires that fork is async-signal-safe, + but our current fork implementation is not. */ + bool multiple_threads = THREAD_GETMEM (THREAD_SELF, header.multiple_threads); + /* Run all the registered preparation handlers. In reverse order. While doing this we build up a list of all the entries. */ struct fork_handler *runp; @@ -109,12 +115,21 @@ __libc_fork (void) break; } - _IO_list_lock (); + /* If we are not running multiple threads, we do not have to + preserve lock state. If fork runs from a signal handler, only + async-signal-safe functions can be used in the child. These data + structures are only used by unsafe functions, so their state does + not matter if fork was called from a signal handler. */ + if (multiple_threads) + { + _IO_list_lock (); - /* Acquire malloc locks. This needs to come last because fork - handlers may use malloc, and the libio list lock has an indirect - malloc dependency as well (via the getdelim function). */ - __malloc_fork_lock_parent (); + /* Acquire malloc locks. This needs to come last because fork + handlers may use malloc, and the libio list lock has an + indirect malloc dependency as well (via the getdelim + function). */ + __malloc_fork_lock_parent (); + } #ifndef NDEBUG pid_t ppid = THREAD_GETMEM (THREAD_SELF, tid); @@ -173,14 +188,18 @@ __libc_fork (void) # endif #endif - /* Release malloc locks. */ - __malloc_fork_unlock_child (); + /* Reset the lock state in the multi-threaded case. */ + if (multiple_threads) + { + /* Release malloc locks. */ + __malloc_fork_unlock_child (); - /* Reset the file list. These are recursive mutexes. */ - fresetlockfiles (); + /* Reset the file list. These are recursive mutexes. */ + fresetlockfiles (); - /* Reset locks in the I/O code. */ - _IO_list_resetlock (); + /* Reset locks in the I/O code. */ + _IO_list_resetlock (); + } /* Reset the lock the dynamic loader uses to protect its data. */ __rtld_lock_initialize (GL(dl_load_lock)); @@ -217,11 +236,15 @@ __libc_fork (void) /* Restore the PID value. */ THREAD_SETMEM (THREAD_SELF, pid, parentpid); - /* Release malloc locks, parent process variant. */ - __malloc_fork_unlock_parent (); + /* Release acquired locks in the multi-threaded case. */ + if (multiple_threads) + { + /* Release malloc locks, parent process variant. */ + __malloc_fork_unlock_parent (); - /* We execute this even if the 'fork' call failed. */ - _IO_list_unlock (); + /* We execute this even if the 'fork' call failed. */ + _IO_list_unlock (); + } /* Run the handlers registered for the parent. */ while (allp != NULL) |