Sign Up
Log In
Log In
or
Sign Up
Places
All Projects
Status Monitor
Collapse sidebar
SUSE:SLE-12-SP4:GA
glibc.33856
glibc-2.29-posix-Use-posix_spawn-on-popen.patch
Overview
Repositories
Revisions
Requests
Users
Attributes
Meta
File glibc-2.29-posix-Use-posix_spawn-on-popen.patch of Package glibc.33856
From 14d0e87d9b8caaa2eca7ca81f1189596671fe4fb Mon Sep 17 00:00:00 2001 From: Adhemerval Zanella <adhemerval.zanella@linaro.org> Date: Wed, 12 Sep 2018 10:32:05 -0300 Subject: [PATCH] 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. --- libio/iopopen.c | 132 ++++++++++++++++++++++++++++++++++++-------------------- 3 files changed, 99 insertions(+), 47 deletions(-) Index: glibc-2.22/libio/iopopen.c =================================================================== --- glibc-2.22.orig/libio/iopopen.c +++ glibc-2.22/libio/iopopen.c @@ -39,7 +39,8 @@ #endif #include <sys/types.h> #include <sys/wait.h> -#include <kernel-features.h> +#include <spawn.h> +#include <paths.h> #ifndef _IO_fork #ifdef _LIBC @@ -105,6 +106,51 @@ 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, _IO_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 ((_IO_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; +} + _IO_FILE * _IO_new_proc_open (fp, command, mode) _IO_FILE *fp; @@ -112,9 +158,11 @@ _IO_new_proc_open (fp, command, mode) const char *mode; { int read_or_write; + /* These are indexes for pipe_fds. */ int parent_end, child_end; int pipe_fds[2]; - _IO_pid_t child_pid; + int child_pipe_fd; + bool spawn_ok; int do_read = 0; int do_write = 0; @@ -157,74 +205,61 @@ _IO_new_proc_open (fp, command, 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 = _IO_fork (); - if (child_pid == 0) - { - int child_std_end = do_read ? 1 : 0; - struct _IO_proc_file *p; - - if (child_end != child_std_end) - _IO_dup2 (child_end, child_std_end); -#ifdef O_CLOEXEC - 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); -#endif - /* 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 ((_IO_FILE *) p); - - /* 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) - _IO_close (fd); - } - - _IO_execl ("/bin/sh", "sh", "-c", command, (char *) 0); - _IO__exit (127); - } - _IO_close (child_end); - if (child_pid < 0) + posix_spawn_file_actions_t fa; + /* posix_spawn_file_actions_init does not fail. */ + __posix_spawn_file_actions_init (&fa); + + /* 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) { - _IO_close (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); - - _IO_fileno (fp) = parent_end; + if (__posix_spawn_file_actions_adddup2 (&fa, pipe_fds[child_end], + child_pipe_fd) != 0) + goto spawn_failure; - /* 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; }
Locations
Projects
Search
Status Monitor
Help
OpenBuildService.org
Documentation
API Documentation
Code of Conduct
Contact
Support
@OBShq
Terms
openSUSE Build Service is sponsored by
The Open Build Service is an
openSUSE project
.
Sign Up
Log In
Places
Places
All Projects
Status Monitor
