This is the mail archive of the
glibc-cvs@sourceware.org
mailing list for the glibc project.
GNU C Library master sources branch master updated. glibc-2.28.9000-337-g5fb7fc9
- From: azanella at sourceware dot org
- To: glibc-cvs at sourceware dot org
- Date: 30 Nov 2018 21:09:59 -0000
- Subject: GNU C Library master sources branch master updated. glibc-2.28.9000-337-g5fb7fc9
This is an automated email from the git hooks/post-receive script. It was
generated because a ref change was pushed to the repository containing
the project "GNU C Library master sources".
The branch, master has been updated
via 5fb7fc96350575c9adb1316833e48ca11553be49 (commit)
via 14d0e87d9b8caaa2eca7ca81f1189596671fe4fb (commit)
from e5d262effe3a87164308a3f37e61b32d0348692a (commit)
Those revisions listed above that are new to this repository have
not appeared on any other notification email; so we list those
revisions in full, below.
- Log -----------------------------------------------------------------
http://sourceware.org/git/gitweb.cgi?p=glibc.git;a=commitdiff;h=5fb7fc96350575c9adb1316833e48ca11553be49
commit 5fb7fc96350575c9adb1316833e48ca11553be49
Author: Adhemerval Zanella <adhemerval.zanella@linaro.org>
Date: Wed Oct 24 16:29:38 2018 -0300
posix: Use posix_spawn on system
This patch uses posix_spawn on system implementation. On Linux this has
the advantage of much lower memory consumption (usually 32 Kb minimum for
the mmap stack area).
Although POSIX does not require, glibc system implementation aims to be
thread and cancellation safe. The cancellation code is moved to generic
implementation and enabled iff SIGCANCEL is defined (similar on how the
cancellation handler is enabled on nptl-init.c).
Checked on x86_64-linux-gnu, i686-linux-gnu, aarch64-linux-gnu,
arm-linux-gnueabihf, and powerpc64le-linux-gnu.
* sysdeps/unix/sysv/linux/spawni.c (__spawni_child): Use
__sigismember instead of sigismember.
* sysdeps/posix/system.c [SIGCANCEL] (cancel_handler_args,
cancel_handler): New definitions.
(CLEANUP_HANDLER, CLEANUP_RESET): Likewise.
(DO_LOCK, DO_UNLOCK, INIT_LOCK, ADD_REF, SUB_REF): Remove.
(do_system): Use posix_spawn instead of fork and execl and remove
reentracy code.
* sysdeps/generic/not-errno.h (__kill_noerrno): New prototype.
* sysdeps/unix/sysv/linux/not-errno.h (__kill_noerrno): Likewise.
* sysdeps/unix/sysv/linux/ia64/system.c: Remove file.
* sysdeps/unix/sysv/linux/s390/system.c: Likewise.
* sysdeps/unix/sysv/linux/sparc/system.c: Likewise.
* sysdeps/unix/sysv/linux/system.c: Likewise.
diff --git a/ChangeLog b/ChangeLog
index 70bd1a3..a8590b2 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,18 @@
2018-11-28 Adhemerval Zanella <adhemerval.zanella@linaro.org>
+ * sysdeps/unix/sysv/linux/spawni.c (__spawni_child): Use
+ __sigismember instead of sigismember.
+ * sysdeps/posix/system.c [SIGCANCEL] (cancel_handler_args,
+ cancel_handler): New definitions.
+ (do_system): Use posix_spawn instead of fork and execl and remove
+ reentracy code.
+ * sysdeps/generic/not-errno.h (__kill_noerrno): New prototype.
+ * sysdeps/unix/sysv/linux/not-errno.h (__kill_noerrno): Likewise.
+ * sysdeps/unix/sysv/linux/ia64/system.c: Remove file.
+ * sysdeps/unix/sysv/linux/s390/system.c: Likewise.
+ * sysdeps/unix/sysv/linux/sparc/system.c: Likewise.
+ * sysdeps/unix/sysv/linux/system.c: Likewise.
+
[BZ #22834]
[BZ #17490]
* NEWS: Add new semantic for atfork with popen and system.
diff --git a/sysdeps/generic/not-errno.h b/sysdeps/generic/not-errno.h
index 93617a3..0fd66b5 100644
--- a/sysdeps/generic/not-errno.h
+++ b/sysdeps/generic/not-errno.h
@@ -17,3 +17,5 @@
<http://www.gnu.org/licenses/>. */
extern __typeof (__access) __access_noerrno attribute_hidden;
+
+extern __typeof (__kill) __kill_noerrno attribute_hidden;
diff --git a/sysdeps/posix/system.c b/sysdeps/posix/system.c
index d759443..8a51a6b 100644
--- a/sysdeps/posix/system.c
+++ b/sysdeps/posix/system.c
@@ -17,20 +17,36 @@
#include <errno.h>
#include <signal.h>
-#include <stddef.h>
#include <stdlib.h>
#include <unistd.h>
+#include <sigsetops.h>
+#include <spawn.h>
+#include <pthread.h>
#include <sys/types.h>
#include <sys/wait.h>
-#include <libc-lock.h>
-#include <sysdep-cancel.h>
-#include <sigsetops.h>
+#include <stdio.h>
+#include <libc-lock.h>
+#include <not-errno.h>
+#include <not-cancel.h>
+#include <internal-signals.h>
#define SHELL_PATH "/bin/sh" /* Path of the shell. */
#define SHELL_NAME "sh" /* Name to give it. */
+/* This system implementation aims to be thread-safe, which requires to
+ restore the signal dispositions for SIGINT and SIGQUIT correctly and to
+ deal with cancellation by terminating the child process.
+
+ The signal disposition restoration on the single-thread case is
+ straighfoward. For multithreaded case, a reference-counter with a lock
+ is used, so the first thread will set the SIGINT/SIGQUIT dispositions and
+ last thread will restore them.
+
+ Cancellation handling is done with thread cancellation clean-up handlers
+ on waitpid call. */
+
#ifdef _LIBC_REENTRANT
static struct sigaction intr, quit;
static int sa_refcntr;
@@ -50,17 +66,45 @@ __libc_lock_define_initialized (static, lock);
#endif
+#if defined(_LIBC_REENTRANT) && defined(SIGCANCEL)
+struct cancel_handler_args
+{
+ struct sigaction *quit;
+ struct sigaction *intr;
+ pid_t pid;
+};
+
+static void
+cancel_handler (void *arg)
+{
+ struct cancel_handler_args *args = (struct cancel_handler_args *) (arg);
+
+ __kill_noerrno (args->pid, SIGKILL);
+
+ TEMP_FAILURE_RETRY (__waitpid_nocancel (args->pid, NULL, 0));
+
+ DO_LOCK ();
+ if (SUB_REF () == 0)
+ {
+ __sigaction (SIGQUIT, args->quit, NULL);
+ __sigaction (SIGINT, args->intr, NULL);
+ }
+ DO_UNLOCK ();
+}
+#endif
+
/* Execute LINE as a shell command, returning its status. */
static int
do_system (const char *line)
{
- int status, save;
+ int status;
pid_t pid;
struct sigaction sa;
#ifndef _LIBC_REENTRANT
struct sigaction intr, quit;
#endif
sigset_t omask;
+ sigset_t reset;
sa.sa_handler = SIG_IGN;
sa.sa_flags = 0;
@@ -69,105 +113,72 @@ do_system (const char *line)
DO_LOCK ();
if (ADD_REF () == 0)
{
- if (__sigaction (SIGINT, &sa, &intr) < 0)
- {
- (void) SUB_REF ();
- goto out;
- }
- if (__sigaction (SIGQUIT, &sa, &quit) < 0)
- {
- save = errno;
- (void) SUB_REF ();
- goto out_restore_sigint;
- }
+ /* sigaction can not fail with SIGINT/SIGQUIT used with SIG_IGN. */
+ __sigaction (SIGINT, &sa, &intr);
+ __sigaction (SIGQUIT, &sa, &quit);
}
DO_UNLOCK ();
- /* We reuse the bitmap in the 'sa' structure. */
__sigaddset (&sa.sa_mask, SIGCHLD);
- save = errno;
- if (__sigprocmask (SIG_BLOCK, &sa.sa_mask, &omask) < 0)
+ /* sigprocmask can not fail with SIG_BLOCK used with valid input
+ arguments. */
+ __sigprocmask (SIG_BLOCK, &sa.sa_mask, &omask);
+
+ __sigemptyset (&reset);
+ if (intr.sa_handler != SIG_IGN)
+ __sigaddset(&reset, SIGINT);
+ if (quit.sa_handler != SIG_IGN)
+ __sigaddset(&reset, SIGQUIT);
+
+ posix_spawnattr_t spawn_attr;
+ /* None of the posix_spawnattr_* function returns an error, including
+ posix_spawnattr_setflags for the follow specific usage (using valid
+ flags). */
+ __posix_spawnattr_init (&spawn_attr);
+ __posix_spawnattr_setsigmask (&spawn_attr, &omask);
+ __posix_spawnattr_setsigdefault (&spawn_attr, &reset);
+ __posix_spawnattr_setflags (&spawn_attr,
+ POSIX_SPAWN_SETSIGDEF | POSIX_SPAWN_SETSIGMASK);
+
+ status = __posix_spawn (&pid, SHELL_PATH, 0, &spawn_attr,
+ (char *const[]){ (char*) SHELL_NAME,
+ (char*) "-c",
+ (char *) line, NULL },
+ __environ);
+ __posix_spawnattr_destroy (&spawn_attr);
+
+ if (status == 0)
{
-#ifndef _LIBC
- if (errno == ENOSYS)
- __set_errno (save);
- else
-#endif
- {
- DO_LOCK ();
- if (SUB_REF () == 0)
- {
- save = errno;
- (void) __sigaction (SIGQUIT, &quit, (struct sigaction *) NULL);
- out_restore_sigint:
- (void) __sigaction (SIGINT, &intr, (struct sigaction *) NULL);
- __set_errno (save);
- }
- out:
- DO_UNLOCK ();
- return -1;
- }
- }
-
-#ifdef CLEANUP_HANDLER
- CLEANUP_HANDLER;
-#endif
-
-#ifdef FORK
- pid = FORK ();
-#else
- pid = __fork ();
+ /* Cancellation results in cleanup handlers running as exceptions in
+ the block where they were installed, so it is safe to reference
+ stack variable allocate in the broader scope. */
+#if defined(_LIBC_REENTRANT) && defined(SIGCANCEL)
+ struct cancel_handler_args cancel_args =
+ {
+ .quit = &quit,
+ .intr = &intr,
+ .pid = pid
+ };
+ __libc_cleanup_region_start (1, cancel_handler, &cancel_args);
#endif
- if (pid == (pid_t) 0)
- {
- /* Child side. */
- const char *new_argv[4];
- new_argv[0] = SHELL_NAME;
- new_argv[1] = "-c";
- new_argv[2] = line;
- new_argv[3] = NULL;
-
- /* Restore the signals. */
- (void) __sigaction (SIGINT, &intr, (struct sigaction *) NULL);
- (void) __sigaction (SIGQUIT, &quit, (struct sigaction *) NULL);
- (void) __sigprocmask (SIG_SETMASK, &omask, (sigset_t *) NULL);
- INIT_LOCK ();
-
- /* Exec the shell. */
- (void) __execve (SHELL_PATH, (char *const *) new_argv, __environ);
- _exit (127);
- }
- else if (pid < (pid_t) 0)
- /* The fork failed. */
- status = -1;
- else
- /* Parent side. */
- {
/* Note the system() is a cancellation point. But since we call
waitpid() which itself is a cancellation point we do not
have to do anything here. */
if (TEMP_FAILURE_RETRY (__waitpid (pid, &status, 0)) != pid)
status = -1;
- }
-
-#ifdef CLEANUP_HANDLER
- CLEANUP_RESET;
+#if defined(_LIBC_REENTRANT) && defined(SIGCANCEL)
+ __libc_cleanup_region_end (0);
#endif
+ }
- save = errno;
DO_LOCK ();
- if ((SUB_REF () == 0
- && (__sigaction (SIGINT, &intr, (struct sigaction *) NULL)
- | __sigaction (SIGQUIT, &quit, (struct sigaction *) NULL)) != 0)
- || __sigprocmask (SIG_SETMASK, &omask, (sigset_t *) NULL) != 0)
+ if (SUB_REF () == 0)
{
-#ifndef _LIBC
- /* glibc cannot be used on systems without waitpid. */
- if (errno == ENOSYS)
- __set_errno (save);
- else
-#endif
- status = -1;
+ /* sigaction can not fail with SIGINT/SIGQUIT used with old
+ disposition. Same applies for sigprocmask. */
+ __sigaction (SIGINT, &intr, NULL);
+ __sigaction (SIGQUIT, &quit, NULL);
+ __sigprocmask (SIG_SETMASK, &omask, NULL);
}
DO_UNLOCK ();
diff --git a/sysdeps/unix/sysv/linux/ia64/system.c b/sysdeps/unix/sysv/linux/ia64/system.c
deleted file mode 100644
index d09fefe..0000000
--- a/sysdeps/unix/sysv/linux/ia64/system.c
+++ /dev/null
@@ -1,30 +0,0 @@
-/* Copyright (C) 2002-2018 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; if not, see
- <http://www.gnu.org/licenses/>. */
-
-/* We have to and actually can handle cancelable system(). The big
- problem: we have to kill the child process if necessary. To do
- this a cleanup handler has to be registered and is has to be able
- to find the PID of the child. The main problem is to reliable have
- the PID when needed. It is not necessary for the parent thread to
- return. It might still be in the kernel when the cancellation
- request comes. Therefore we have to use the clone() calls ability
- to have the kernel write the PID into the user-level variable. */
-#define FORK() \
- INLINE_SYSCALL (clone2, 6, CLONE_PARENT_SETTID | SIGCHLD, NULL, 0, \
- &pid, NULL, NULL)
-
-#include <sysdeps/unix/sysv/linux/system.c>
diff --git a/sysdeps/unix/sysv/linux/not-errno.h b/sysdeps/unix/sysv/linux/not-errno.h
index 106ba5c..b2f72cf 100644
--- a/sysdeps/unix/sysv/linux/not-errno.h
+++ b/sysdeps/unix/sysv/linux/not-errno.h
@@ -16,6 +16,9 @@
License along with the GNU C Library; if not, see
<http://www.gnu.org/licenses/>. */
+#include <sysdep.h>
+#include <fcntl.h>
+
/* This function is used on maybe_enable_malloc_check (elf/dl-tunables.c)
and to avoid having to build/use multiple versions if stack protection
in enabled it is defined as inline. */
@@ -33,3 +36,14 @@ __access_noerrno (const char *pathname, int mode)
return INTERNAL_SYSCALL_ERRNO (res, err);
return 0;
}
+
+static inline int
+__kill_noerrno (pid_t pid, int sig)
+{
+ int res;
+ INTERNAL_SYSCALL_DECL (err);
+ res = INTERNAL_SYSCALL_CALL (kill, err, pid, sig);
+ if (INTERNAL_SYSCALL_ERROR_P (res, err))
+ return INTERNAL_SYSCALL_ERRNO (res, err);
+ return 0;
+}
diff --git a/sysdeps/unix/sysv/linux/s390/system.c b/sysdeps/unix/sysv/linux/s390/system.c
deleted file mode 100644
index d8ef461..0000000
--- a/sysdeps/unix/sysv/linux/s390/system.c
+++ /dev/null
@@ -1,29 +0,0 @@
-/* Copyright (C) 2003-2018 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; if not, see
- <http://www.gnu.org/licenses/>. */
-
-/* We have to and actually can handle cancelable system(). The big
- problem: we have to kill the child process if necessary. To do
- this a cleanup handler has to be registered and is has to be able
- to find the PID of the child. The main problem is to reliable have
- the PID when needed. It is not necessary for the parent thread to
- return. It might still be in the kernel when the cancellation
- request comes. Therefore we have to use the clone() calls ability
- to have the kernel write the PID into the user-level variable. */
-#define FORK() \
- INLINE_SYSCALL (clone, 3, 0, CLONE_PARENT_SETTID | SIGCHLD, &pid)
-
-#include "../system.c"
diff --git a/sysdeps/unix/sysv/linux/sparc/system.c b/sysdeps/unix/sysv/linux/sparc/system.c
deleted file mode 100644
index 1f65c83..0000000
--- a/sysdeps/unix/sysv/linux/sparc/system.c
+++ /dev/null
@@ -1,29 +0,0 @@
-/* Copyright (C) 2003-2018 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; if not, see
- <http://www.gnu.org/licenses/>. */
-
-/* We have to and actually can handle cancelable system(). The big
- problem: we have to kill the child process if necessary. To do
- this a cleanup handler has to be registered and is has to be able
- to find the PID of the child. The main problem is to reliable have
- the PID when needed. It is not necessary for the parent thread to
- return. It might still be in the kernel when the cancellation
- request comes. Therefore we have to use the clone() calls ability
- to have the kernel write the PID into the user-level variable. */
-#define FORK() \
- INLINE_CLONE_SYSCALL (CLONE_PARENT_SETTID | SIGCHLD, 0, &pid, NULL, NULL)
-
-#include "../system.c"
diff --git a/sysdeps/unix/sysv/linux/spawni.c b/sysdeps/unix/sysv/linux/spawni.c
index 9f3a137..dbb6cdd 100644
--- a/sysdeps/unix/sysv/linux/spawni.c
+++ b/sysdeps/unix/sysv/linux/spawni.c
@@ -138,11 +138,11 @@ __spawni_child (void *arguments)
for (int sig = 1; sig < _NSIG; ++sig)
{
if ((attr->__flags & POSIX_SPAWN_SETSIGDEF)
- && sigismember (&attr->__sd, sig))
+ && __sigismember (&attr->__sd, sig))
{
sa.sa_handler = SIG_DFL;
}
- else if (sigismember (&hset, sig))
+ else if (__sigismember (&hset, sig))
{
if (__is_internal_signal (sig))
sa.sa_handler = SIG_IGN;
diff --git a/sysdeps/unix/sysv/linux/system.c b/sysdeps/unix/sysv/linux/system.c
deleted file mode 100644
index 7cc68a1..0000000
--- a/sysdeps/unix/sysv/linux/system.c
+++ /dev/null
@@ -1,76 +0,0 @@
-/* Copyright (C) 2002-2018 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; if not, see
- <http://www.gnu.org/licenses/>. */
-
-#include <sched.h>
-#include <signal.h>
-#include <string.h> /* For the real memset prototype. */
-#include <sysdep.h>
-#include <unistd.h>
-#include <sys/wait.h>
-#include <libc-lock.h>
-
-/* We have to and actually can handle cancelable system(). The big
- problem: we have to kill the child process if necessary. To do
- this a cleanup handler has to be registered and is has to be able
- to find the PID of the child. The main problem is to reliable have
- the PID when needed. It is not necessary for the parent thread to
- return. It might still be in the kernel when the cancellation
- request comes. Therefore we have to use the clone() calls ability
- to have the kernel write the PID into the user-level variable. */
-#ifndef FORK
-# define FORK() \
- INLINE_SYSCALL (clone, 3, CLONE_PARENT_SETTID | SIGCHLD, 0, &pid)
-#endif
-
-#ifdef _LIBC_REENTRANT
-static void cancel_handler (void *arg);
-
-# define CLEANUP_HANDLER \
- __libc_cleanup_region_start (1, cancel_handler, &pid)
-
-# define CLEANUP_RESET \
- __libc_cleanup_region_end (0)
-#endif
-
-
-/* Linux has waitpid(), so override the generic unix version. */
-#include <sysdeps/posix/system.c>
-
-
-#ifdef _LIBC_REENTRANT
-/* The cancellation handler. */
-static void
-cancel_handler (void *arg)
-{
- pid_t child = *(pid_t *) arg;
-
- INTERNAL_SYSCALL_DECL (err);
- INTERNAL_SYSCALL (kill, err, 2, child, SIGKILL);
-
- TEMP_FAILURE_RETRY (__waitpid (child, NULL, 0));
-
- DO_LOCK ();
-
- if (SUB_REF () == 0)
- {
- (void) __sigaction (SIGQUIT, &quit, (struct sigaction *) NULL);
- (void) __sigaction (SIGINT, &intr, (struct sigaction *) NULL);
- }
-
- DO_UNLOCK ();
-}
-#endif
http://sourceware.org/git/gitweb.cgi?p=glibc.git;a=commitdiff;h=14d0e87d9b8caaa2eca7ca81f1189596671fe4fb
commit 14d0e87d9b8caaa2eca7ca81f1189596671fe4fb
Author: Adhemerval Zanella <adhemerval.zanella@linaro.org>
Date: Wed Sep 12 10:32:05 2018 -0300
posix: Use posix_spawn on popen
This patch uses posix_spawn on popen instead of fork and execl. On Linux
this has the advantage of much lower memory consumption (usually 32 Kb
minimum for the mmap stack area).
Two issues are also fixed with this change:
* BZ#17490: although POSIX pthread_atfork description only list 'fork'
as the function that should execute the atfork handlers, popen
description states that:
'[...] shall be *as if* a child process were created within the popen()
call using the fork() function [...]'
Other libc/system seems to follow the idea atfork handlers should not be
executed for popen:
libc/system | run atfork handles | notes
------------|----------------------|---------------------------------------
Freebsd | no | uses vfork
Solaris 11 | no |
MacOSX 11 | no | implemented through posix_spawn syscall
------------|----------------------|----------------------------------------
Similar to posix_spawn and system, popen idea is to spawn a different
binary so all the POSIX rationale to run the atfork handlers to avoid
internal process inconsistency is not really required and in some cases
might be unsafe.
* BZ#22834: the described scenario, where the forked process might access
invalid memory due an inconsistent state in multithreaded environment,
should not happen because posix_spawn does not access the affected
data structure (proc_file_chain).
Checked on x86_64-linux-gnu and i686-linux-gnu.
[BZ #22834]
[BZ #17490]
* NEWS: Add new semantic for atfork with popen and system.
* libio/iopopen.c (_IO_new_proc_open): use posix_spawn instead of
fork and execl.
diff --git a/ChangeLog b/ChangeLog
index 09f7168..70bd1a3 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,11 @@
+2018-11-28 Adhemerval Zanella <adhemerval.zanella@linaro.org>
+
+ [BZ #22834]
+ [BZ #17490]
+ * NEWS: Add new semantic for atfork with popen and system.
+ * libio/iopopen.c (_IO_new_proc_open): use posix_spawn instead of
+ fork and execl.
+
2018-11-30 Tulio Magno Quites Machado Filho <tuliom@linux.ibm.com>
[BZ #23690]
diff --git a/NEWS b/NEWS
index 1098be1..8483dcf 100644
--- a/NEWS
+++ b/NEWS
@@ -35,6 +35,12 @@ Major new features:
different directory. This is a GNU extension and similar to the
Solaris function of the same name.
+* The popen and system do not run atfork handlers anymore (BZ#17490).
+ Although it is a possible POSIX violation, the POSIX rationale in
+ pthread_atfork documentation regarding atfork handlers is to handle
+ incosistent mutex state after fork call in multithread environment.
+ In both popen and system there is no direct access to user-defined mutexes.
+
Deprecated and removed features, and other changes affecting compatibility:
* The glibc.tune tunable namespace has been renamed to glibc.cpu and the
diff --git a/libio/iopopen.c b/libio/iopopen.c
index 2eff45b..c768295 100644
--- a/libio/iopopen.c
+++ b/libio/iopopen.c
@@ -34,7 +34,8 @@
#include <not-cancel.h>
#include <sys/types.h>
#include <sys/wait.h>
-#include <kernel-features.h>
+#include <spawn.h>
+#include <paths.h>
struct _IO_proc_file
{
@@ -59,13 +60,60 @@ unlock (void *not_used)
}
#endif
+/* POSIX states popen shall ensure that any streams from previous popen()
+ calls that remain open in the parent process should be closed in the new
+ child process.
+ To avoid a race-condition between checking which file descriptors need to
+ be close (by transversing the proc_file_chain list) and the insertion of a
+ new one after a successful posix_spawn this function should be called
+ with proc_file_chain_lock acquired. */
+static bool
+spawn_process (posix_spawn_file_actions_t *fa, FILE *fp, const char *command,
+ int do_cloexec, int pipe_fds[2], int parent_end, int child_end,
+ int child_pipe_fd)
+{
+
+ for (struct _IO_proc_file *p = proc_file_chain; p; p = p->next)
+ {
+ int fd = _IO_fileno ((FILE *) p);
+
+ /* If any stream from previous popen() calls has fileno
+ child_pipe_fd, it has been already closed by the adddup2 action
+ above. */
+ if (fd != child_pipe_fd
+ && __posix_spawn_file_actions_addclose (fa, fd) != 0)
+ return false;
+ }
+
+ if (__posix_spawn (&((_IO_proc_file *) fp)->pid, _PATH_BSHELL, fa, 0,
+ (char *const[]){ (char*) "sh", (char*) "-c",
+ (char *) command, NULL }, __environ) != 0)
+ return false;
+
+ __close_nocancel (pipe_fds[child_end]);
+
+ if (!do_cloexec)
+ /* Undo the effects of the pipe2 call which set the
+ close-on-exec flag. */
+ __fcntl (pipe_fds[parent_end], F_SETFD, 0);
+
+ _IO_fileno (fp) = pipe_fds[parent_end];
+
+ ((_IO_proc_file *) fp)->next = proc_file_chain;
+ proc_file_chain = (_IO_proc_file *) fp;
+
+ return true;
+}
+
FILE *
_IO_new_proc_open (FILE *fp, const char *command, const char *mode)
{
int read_or_write;
+ /* These are indexes for pipe_fds. */
int parent_end, child_end;
int pipe_fds[2];
- pid_t child_pid;
+ int child_pipe_fd;
+ bool spawn_ok;
int do_read = 0;
int do_write = 0;
@@ -108,72 +156,62 @@ _IO_new_proc_open (FILE *fp, const char *command, const char *mode)
if (do_read)
{
- parent_end = pipe_fds[0];
- child_end = pipe_fds[1];
+ parent_end = 0;
+ child_end = 1;
read_or_write = _IO_NO_WRITES;
+ child_pipe_fd = 1;
}
else
{
- parent_end = pipe_fds[1];
- child_end = pipe_fds[0];
+ parent_end = 1;
+ child_end = 0;
read_or_write = _IO_NO_READS;
+ child_pipe_fd = 0;
}
- ((_IO_proc_file *) fp)->pid = child_pid = __fork ();
- if (child_pid == 0)
- {
- int child_std_end = do_read ? 1 : 0;
- struct _IO_proc_file *p;
-
- if (child_end != child_std_end)
- __dup2 (child_end, child_std_end);
- else
- /* The descriptor is already the one we will use. But it must
- not be marked close-on-exec. Undo the effects. */
- __fcntl (child_end, F_SETFD, 0);
- /* POSIX.2: "popen() shall ensure that any streams from previous
- popen() calls that remain open in the parent process are closed
- in the new child process." */
- for (p = proc_file_chain; p; p = p->next)
- {
- int fd = _IO_fileno ((FILE *) p);
+ posix_spawn_file_actions_t fa;
+ /* posix_spawn_file_actions_init does not fail. */
+ __posix_spawn_file_actions_init (&fa);
- /* If any stream from previous popen() calls has fileno
- child_std_end, it has been already closed by the dup2 syscall
- above. */
- if (fd != child_std_end)
- __close_nocancel (fd);
- }
-
- execl ("/bin/sh", "sh", "-c", command, (char *) 0);
- _exit (127);
- }
- __close_nocancel (child_end);
- if (child_pid < 0)
+ /* The descriptor is already the one the child will use. In this case
+ it must be moved to another one otherwise, there is no safe way to
+ remove the close-on-exec flag in the child without creating a FD leak
+ race in the parent. */
+ if (pipe_fds[child_end] == child_pipe_fd)
{
- __close_nocancel (parent_end);
- return NULL;
+ int tmp = __fcntl (child_pipe_fd, F_DUPFD_CLOEXEC, 0);
+ if (tmp < 0)
+ goto spawn_failure;
+ __close_nocancel (pipe_fds[child_end]);
+ pipe_fds[child_end] = tmp;
}
- if (!do_cloexec)
- /* Undo the effects of the pipe2 call which set the
- close-on-exec flag. */
- __fcntl (parent_end, F_SETFD, 0);
+ if (__posix_spawn_file_actions_adddup2 (&fa, pipe_fds[child_end],
+ child_pipe_fd) != 0)
+ goto spawn_failure;
- _IO_fileno (fp) = parent_end;
-
- /* Link into proc_file_chain. */
#ifdef _IO_MTSAFE_IO
_IO_cleanup_region_start_noarg (unlock);
_IO_lock_lock (proc_file_chain_lock);
#endif
- ((_IO_proc_file *) fp)->next = proc_file_chain;
- proc_file_chain = (_IO_proc_file *) fp;
+ spawn_ok = spawn_process (&fa, fp, command, do_cloexec, pipe_fds,
+ parent_end, child_end, child_pipe_fd);
#ifdef _IO_MTSAFE_IO
_IO_lock_unlock (proc_file_chain_lock);
_IO_cleanup_region_end (0);
#endif
+ __posix_spawn_file_actions_destroy (&fa);
+
+ if (!spawn_ok)
+ {
+ spawn_failure:
+ __close_nocancel (pipe_fds[child_end]);
+ __close_nocancel (pipe_fds[parent_end]);
+ __set_errno (ENOMEM);
+ return NULL;
+ }
+
_IO_mask_flags (fp, read_or_write, _IO_NO_READS|_IO_NO_WRITES);
return fp;
}
-----------------------------------------------------------------------
Summary of changes:
ChangeLog | 21 ++++
NEWS | 6 +
libio/iopopen.c | 134 +++++++++++++++--------
sysdeps/generic/not-errno.h | 2 +
sysdeps/posix/system.c | 189 +++++++++++++++++---------------
sysdeps/unix/sysv/linux/ia64/system.c | 30 -----
sysdeps/unix/sysv/linux/not-errno.h | 14 +++
sysdeps/unix/sysv/linux/s390/system.c | 29 -----
sysdeps/unix/sysv/linux/sparc/system.c | 29 -----
sysdeps/unix/sysv/linux/spawni.c | 4 +-
sysdeps/unix/sysv/linux/system.c | 76 -------------
11 files changed, 231 insertions(+), 303 deletions(-)
delete mode 100644 sysdeps/unix/sysv/linux/ia64/system.c
delete mode 100644 sysdeps/unix/sysv/linux/s390/system.c
delete mode 100644 sysdeps/unix/sysv/linux/sparc/system.c
delete mode 100644 sysdeps/unix/sysv/linux/system.c
hooks/post-receive
--
GNU C Library master sources