diff options
author | Adhemerval Zanella <adhemerval.zanella@linaro.org> | 2016-09-16 17:44:50 -0300 |
---|---|---|
committer | Adhemerval Zanella <adhemerval.zanella@linaro.com> | 2016-09-28 14:07:35 -0700 |
commit | e83be730910c341f2f02ccc207b0586bb04fc21a (patch) | |
tree | 6f8ff0de28073a7706be43359fa45fa50ea1b3f3 | |
parent | 4b4d4056bb154603f36c6f8845757c1012758158 (diff) | |
download | glibc-e83be730910c341f2f02ccc207b0586bb04fc21a.tar glibc-e83be730910c341f2f02ccc207b0586bb04fc21a.tar.gz glibc-e83be730910c341f2f02ccc207b0586bb04fc21a.tar.bz2 glibc-e83be730910c341f2f02ccc207b0586bb04fc21a.zip |
posix: Fix open file action for posix_spawn on Linux
On posix_spawn open file action (issued by posix_spawn_file_actions_addopen)
POSIX states that if fildes was already an open file descriptor, it shall be
closed before the new file is openedi [1]. This avoid pontential issues when
posix_spawn plus addopen action is called with the process already at maximum
number of file descriptor opened and also for multiple actions on single-open
special paths (like /dev/watchdog).
This fixes its behavior on Linux posix_spawn implementation and also adds
a tests to check for its behavior.
Checked on x86_64.
* posix/Makefile (tests): Add tst-spawn3.
* posix/tst-spawn3.c: New file.
* sysdeps/unix/sysv/linux/spawni.c (__spawni_child): Close file descriptor
if it is already opened for open action.
[1] http://pubs.opengroup.org/onlinepubs/9699919799/functions/posix_spawn_file_actions_addclose.html
-rw-r--r-- | ChangeLog | 5 | ||||
-rw-r--r-- | posix/Makefile | 2 | ||||
-rw-r--r-- | posix/tst-spawn3.c | 189 | ||||
-rw-r--r-- | sysdeps/unix/sysv/linux/spawni.c | 8 |
4 files changed, 203 insertions, 1 deletions
@@ -497,6 +497,11 @@ 2016-09-20 Adhemerval Zanella <adhemerval.zanella@linaro.org> + * posix/Makefile (tests): Add tst-spawn3. + * posix/tst-spawn3.c: New file. + * sysdeps/unix/sysv/linux/spawni.c (__spawni_child): Close file descriptor + if it is already opened for open action. + * sysdeps/unix/sysv/linux/spawni.c (__spawnix): Correctly block and unblock all signals when executing the clone vfork child. (SIGALL_SET): Remove macro. diff --git a/posix/Makefile b/posix/Makefile index 3a7719e8c2..586d45bf2a 100644 --- a/posix/Makefile +++ b/posix/Makefile @@ -94,7 +94,7 @@ tests := tstgetopt testfnm runtests runptests \ xtests := bug-ga2 ifeq (yes,$(build-shared)) test-srcs := globtest -tests += wordexp-test tst-exec tst-spawn tst-spawn2 +tests += wordexp-test tst-exec tst-spawn tst-spawn2 tst-spawn3 endif tests-static = tst-exec-static tst-spawn-static tests += $(tests-static) diff --git a/posix/tst-spawn3.c b/posix/tst-spawn3.c new file mode 100644 index 0000000000..7d5ffc6cac --- /dev/null +++ b/posix/tst-spawn3.c @@ -0,0 +1,189 @@ +/* Check posix_spawn add file actions. + 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; if not, see + <http://www.gnu.org/licenses/>. */ + +#include <stdio.h> +#include <spawn.h> +#include <error.h> +#include <errno.h> +#include <stdlib.h> +#include <string.h> +#include <unistd.h> +#include <sys/wait.h> +#include <sys/resource.h> + +static int do_test (void); +#define TEST_FUNCTION do_test () +#include <test-skeleton.c> + +static int +do_test (void) +{ + /* The test checks if posix_spawn open file action close the file descriptor + before opening a new one in case the input file descriptor is already + opened. It does by exhausting all file descriptors on the process before + issue posix_spawn. It then issues a posix_spawn for '/bin/sh echo $$' + and add two rules: + + 1. Redirect stdout to a temporary filepath + 2. Redirect stderr to stdout + + If the implementation does not close the file 1. will fail with + EMFILE. */ + + struct rlimit rl; + int max_fd = 24; + + /* Set maximum number of file descriptor to a low value to avoid open + too many files in environments where RLIMIT_NOFILE is large and to + limit the array size to track the opened file descriptors. */ + + if (getrlimit (RLIMIT_NOFILE, &rl) == -1) + { + printf ("error: getrlimit RLIMIT_NOFILE failed"); + exit (EXIT_FAILURE); + } + + max_fd = (rl.rlim_cur < max_fd ? rl.rlim_cur : max_fd); + rl.rlim_cur = max_fd; + + if (setrlimit (RLIMIT_NOFILE, &rl) == 1) + { + printf ("error: setrlimit RLIMIT_NOFILE to %u failed", max_fd); + exit (EXIT_FAILURE); + } + + /* Exhauste the file descriptor limit with temporary files. */ + int files[max_fd]; + int nfiles = 0; + for (;;) + { + int fd = create_temp_file ("tst-spawn3.", NULL); + if (fd == -1) + { + if (errno != EMFILE) + { + printf ("error: create_temp_file returned -1 with " + "errno != EMFILE\n"); + exit (EXIT_FAILURE); + } + break; + } + files[nfiles++] = fd; + } + + posix_spawn_file_actions_t a; + if (posix_spawn_file_actions_init (&a) != 0) + { + puts ("error: spawn_file_actions_init failed"); + exit (EXIT_FAILURE); + } + + /* Executes a /bin/sh echo $$ 2>&1 > /tmp/tst-spawn3.pid . */ + const char pidfile[] = "/tmp/tst-spawn3.pid"; + if (posix_spawn_file_actions_addopen (&a, STDOUT_FILENO, pidfile, O_WRONLY | + O_CREAT | O_TRUNC, 0644) != 0) + { + puts ("error: spawn_file_actions_addopen failed"); + exit (EXIT_FAILURE); + } + + if (posix_spawn_file_actions_adddup2 (&a, STDOUT_FILENO, STDERR_FILENO) != 0) + { + puts ("error: spawn_file_actions_addclose"); + exit (EXIT_FAILURE); + } + + /* Since execve (called by posix_spawn) might require to open files to + actually execute the shell script, setup to close the temporary file + descriptors. */ + for (int i=0; i<nfiles; i++) + { + if (posix_spawn_file_actions_addclose (&a, files[i])) + { + printf ("error: posix_spawn_file_actions_addclose failed"); + exit (EXIT_FAILURE); + } + } + + char *spawn_argv[] = { (char *) _PATH_BSHELL, (char *) "-c", + (char *) "echo $$", NULL }; + pid_t pid; + if (posix_spawn (&pid, _PATH_BSHELL, &a, NULL, spawn_argv, NULL) != 0) + { + puts ("error: posix_spawn failed"); + exit (EXIT_FAILURE); + } + + int status; + int err = waitpid (pid, &status, 0); + if (err != pid) + { + puts ("error: waitpid failed"); + exit (EXIT_FAILURE); + } + + /* Close the temporary files descriptor so it can check posix_spawn + output. */ + for (int i=0; i<nfiles; i++) + { + if (close (files[i])) + { + printf ("error: close failed\n"); + exit (EXIT_FAILURE); + } + } + + int pidfd = open (pidfile, O_RDONLY); + if (pidfd == -1) + { + printf ("error: open pidfile failed\n"); + exit (EXIT_FAILURE); + } + + char buf[64]; + ssize_t n; + if ((n = read (pidfd, buf, sizeof (buf))) < 0) + { + printf ("error: read pidfile failed\n"); + exit (EXIT_FAILURE); + } + + unlink (pidfile); + + /* We only expect to read the PID. */ + char *endp; + long int rpid = strtol (buf, &endp, 10); + if (*endp != '\n') + { + printf ("error: didn't parse whole line: \"%s\"\n", buf); + exit (EXIT_FAILURE); + } + if (endp == buf) + { + puts ("error: read empty line"); + exit (EXIT_FAILURE); + } + + if (rpid != pid) + { + printf ("error: found \"%s\", expected PID %ld\n", buf, (long int) pid); + exit (EXIT_FAILURE); + } + + return 0; +} diff --git a/sysdeps/unix/sysv/linux/spawni.c b/sysdeps/unix/sysv/linux/spawni.c index 679534b79d..fda8593287 100644 --- a/sysdeps/unix/sysv/linux/spawni.c +++ b/sysdeps/unix/sysv/linux/spawni.c @@ -219,6 +219,14 @@ __spawni_child (void *arguments) case spawn_do_open: { + /* POSIX states that if fildes was already an open file descriptor, + it shall be closed before the new file is opened. This avoid + pontential issues when posix_spawn plus addopen action is called + with the process already at maximum number of file descriptor + opened and also for multiple actions on single-open special + paths (like /dev/watchdog). */ + close_not_cancel (action->action.open_action.fd); + ret = open_not_cancel (action->action.open_action.path, action->action. open_action.oflag | O_LARGEFILE, |