* [PATCH 2/3] posix: execvpe cleanup
2016-02-19 18:05 [PATCH v2 0/3] posix: Execute file function fixes Adhemerval Zanella
@ 2016-02-19 18:05 ` Adhemerval Zanella
2016-02-20 8:32 ` Mike Frysinger
2016-02-19 18:05 ` [PATCH 1/3] posix: Remove dynamic memory allocation from execl{e,p} Adhemerval Zanella
` (2 subsequent siblings)
3 siblings, 1 reply; 24+ messages in thread
From: Adhemerval Zanella @ 2016-02-19 18:05 UTC (permalink / raw)
To: libc-alpha
This patch removes all the dynamic allocation on execvpe code and
instead use direct stack allocation. This is QoI approach to make
it possible use in scenarios where memory is shared with parent
(vfork or clone with CLONE_VM).
For default process spawn (script file without a shebang), stack
allocation is bounded by NAME_MAX plus PATH_MAX plus 1. Large
file arguments returns an error (ENAMETOOLONG). This differs than
current GLIBC pratice in general, but it used to limit stack
allocation for large inputs. Also, path in PATH environment variable
larger than PATH_MAX are ignored.
The shell direct execution exeception, where execve returns ENOEXEC,
might requires a large stack allocation due large input argument list.
Tested on i686, x86_64, powerpc64le, and aarch64.
* posix/execvpe.c (__execvpe): Remove dynamic allocation.
* posix/Makefile (tests): Add tst-execvpe{1,2,3,4,5,6}.
* posix/tst-execvp1.c (do_test): Use a macro to call execvp.
* posix/tst-execvp2.c (do_test): Likewise.
* posix/tst-execvp3.c (do_test): Likewise.
* posix/tst-execvp4.c (do_test): Likewise.
* posix/tst-execvpe1.c: New file.
* posix/tst-execvpe2.c: Likewise.
* posix/tst-execvpe3.c: Likewise.
* posix/tst-execvpe4.c: Likewise.
* posix/tst-execvpe5.c: Likewise.
* posix/tst-execvpe6.c: Likewise.
---
ChangeLog | 13 +++
posix/Makefile | 3 +
posix/execvpe.c | 238 +++++++++++++++++++++------------------------------
posix/tst-execvp1.c | 6 +-
posix/tst-execvp2.c | 5 +-
posix/tst-execvp3.c | 5 +-
posix/tst-execvp4.c | 6 +-
posix/tst-execvpe1.c | 20 +++++
posix/tst-execvpe2.c | 20 +++++
posix/tst-execvpe3.c | 20 +++++
posix/tst-execvpe4.c | 20 +++++
posix/tst-execvpe5.c | 130 ++++++++++++++++++++++++++++
posix/tst-execvpe6.c | 82 ++++++++++++++++++
13 files changed, 422 insertions(+), 146 deletions(-)
create mode 100644 posix/tst-execvpe1.c
create mode 100644 posix/tst-execvpe2.c
create mode 100644 posix/tst-execvpe3.c
create mode 100644 posix/tst-execvpe4.c
create mode 100644 posix/tst-execvpe5.c
create mode 100644 posix/tst-execvpe6.c
diff --git a/posix/Makefile b/posix/Makefile
index 55f4f31..a2aabcc 100644
--- a/posix/Makefile
+++ b/posix/Makefile
@@ -82,6 +82,8 @@ tests := tstgetopt testfnm runtests runptests \
tst-execv1 tst-execv2 tst-execl1 tst-execl2 \
tst-execve1 tst-execve2 tst-execle1 tst-execle2 \
tst-execvp3 tst-execvp4 tst-rfc3484 tst-rfc3484-2 \
+ tst-execvpe1 tst-execvpe2 tst-execvpe3 tst-execvpe4 \
+ tst-execvpe5 tst-execvpe6 \
tst-rfc3484-3 \
tst-getaddrinfo3 tst-fnmatch2 tst-cpucount tst-cpuset \
bug-getopt1 bug-getopt2 bug-getopt3 bug-getopt4 \
@@ -229,6 +231,7 @@ tstgetopt-ARGS = -a -b -cfoobar --required foobar --optional=bazbug \
tst-exec-ARGS = -- $(host-test-program-cmd)
tst-exec-static-ARGS = $(tst-exec-ARGS)
+tst-execvpe5-ARGS = -- $(host-test-program-cmd)
tst-spawn-ARGS = -- $(host-test-program-cmd)
tst-spawn-static-ARGS = $(tst-spawn-ARGS)
tst-dir-ARGS = `pwd` `cd $(common-objdir)/$(subdir); pwd` `cd $(common-objdir); pwd` $(objpfx)tst-dir
diff --git a/posix/execvpe.c b/posix/execvpe.c
index 61697a7..ff11b28 100644
--- a/posix/execvpe.c
+++ b/posix/execvpe.c
@@ -15,7 +15,6 @@
License along with the GNU C Library; if not, see
<http://www.gnu.org/licenses/>. */
-#include <alloca.h>
#include <unistd.h>
#include <stdarg.h>
#include <stdbool.h>
@@ -23,15 +22,28 @@
#include <string.h>
#include <errno.h>
#include <paths.h>
+#include <confstr.h>
+#include <sys/param.h>
+#ifndef PATH_MAX
+# ifdef MAXPATHLEN
+# define PATH_MAX MAXPATHLEN
+# else
+# define PATH_MAX 1024
+# endif
+#endif
/* The file is accessible but it is not an executable file. Invoke
the shell to interpret it as a script. */
static void
-internal_function
-scripts_argv (const char *file, char *const argv[], int argc, char **new_argv)
+maybe_script_execute (const char *file, char *const argv[], char *const envp[])
{
+ int argc = 0;
+ while (argv[argc++] != NULL)
+ continue;
+
/* Construct an argument list for the shell. */
+ char *new_argv[argc];
new_argv[0] = (char *) _PATH_BSHELL;
new_argv[1] = (char *) file;
while (argc > 1)
@@ -39,6 +51,9 @@ scripts_argv (const char *file, char *const argv[], int argc, char **new_argv)
new_argv[argc] = argv[argc - 1];
--argc;
}
+
+ /* Execute the shell. */
+ __execve (new_argv[0], new_argv, envp);
}
@@ -47,170 +62,109 @@ scripts_argv (const char *file, char *const argv[], int argc, char **new_argv)
int
__execvpe (const char *file, char *const argv[], char *const envp[])
{
+ /* We check the simple case first. */
if (*file == '\0')
{
- /* We check the simple case first. */
__set_errno (ENOENT);
return -1;
}
+ /* Don't search when it contains a slash. */
if (strchr (file, '/') != NULL)
{
- /* Don't search when it contains a slash. */
__execve (file, argv, envp);
if (errno == ENOEXEC)
- {
- /* Count the arguments. */
- int argc = 0;
- while (argv[argc++])
- ;
- size_t len = (argc + 1) * sizeof (char *);
- char **script_argv;
- void *ptr = NULL;
- if (__libc_use_alloca (len))
- script_argv = alloca (len);
- else
- script_argv = ptr = malloc (len);
-
- if (script_argv != NULL)
- {
- scripts_argv (file, argv, argc, script_argv);
- __execve (script_argv[0], script_argv, envp);
-
- free (ptr);
- }
- }
+ maybe_script_execute (file, argv, envp);
+
+ return -1;
}
- else
+
+ const char *path = getenv ("PATH");
+ if (!path)
+ path = CS_PATH;
+ /* Although GLIBC does not enforce NAME_MAX, we set it as the maximum
+ size to avoid unbounded stack allocation. Same applies for
+ PATH_MAX. */
+ size_t file_len = __strnlen (file, NAME_MAX + 1);
+ size_t path_len = __strnlen (path, PATH_MAX - 1) + 1;
+
+ if ((file_len > NAME_MAX)
+ || !__libc_alloca_cutoff (path_len + file_len + 1))
{
- size_t pathlen;
- size_t alloclen = 0;
- char *path = getenv ("PATH");
- if (path == NULL)
- {
- pathlen = confstr (_CS_PATH, (char *) NULL, 0);
- alloclen = pathlen + 1;
- }
- else
- pathlen = strlen (path);
+ errno = ENAMETOOLONG;
+ return -1;
+ }
- size_t len = strlen (file) + 1;
- alloclen += pathlen + len + 1;
+ const char *subp;
+ bool got_eacces = false;
+ for (const char *p = path; ; p = subp)
+ {
+ char buffer[path_len + file_len + 1];
- char *name;
- char *path_malloc = NULL;
- if (__libc_use_alloca (alloclen))
- name = alloca (alloclen);
- else
- {
- path_malloc = name = malloc (alloclen);
- if (name == NULL)
- return -1;
- }
+ subp = __strchrnul (p, ':');
- if (path == NULL)
+ /* PATH is larger than PATH_MAX and thus potentially larger than
+ the stack allocation. */
+ if (subp - p >= path_len)
{
- /* There is no `PATH' in the environment.
- The default search path is the current directory
- followed by the path `confstr' returns for `_CS_PATH'. */
- path = name + pathlen + len + 1;
- path[0] = ':';
- (void) confstr (_CS_PATH, path + 1, pathlen);
+ /* If there is only one path, bail out. */
+ if (!*subp)
+ break;
+ /* Otherwise skip to next one. */
+ continue;
}
- /* Copy the file name at the top. */
- name = (char *) memcpy (name + pathlen + 1, file, len);
- /* And add the slash. */
- *--name = '/';
+ /* Set current path considered plus a '/'. */
+ memcpy (buffer, p, subp - p);
+ buffer[subp - p] = '/';
+ /* And the file to execute. */
+ memcpy (buffer + (subp - p) + (subp > p), file, file_len + 1);
+
+ __execve (buffer, argv, envp);
+
+ if (errno == ENOEXEC)
+ maybe_script_execute (buffer, argv, envp);
- char **script_argv = NULL;
- void *script_argv_malloc = NULL;
- bool got_eacces = false;
- char *p = path;
- do
+ switch (errno)
{
- char *startp;
-
- path = p;
- p = __strchrnul (path, ':');
-
- if (p == path)
- /* Two adjacent colons, or a colon at the beginning or the end
- of `PATH' means to search the current directory. */
- startp = name + 1;
- else
- startp = (char *) memcpy (name - (p - path), path, p - path);
-
- /* Try to execute this name. If it works, execve will not return. */
- __execve (startp, argv, envp);
-
- if (errno == ENOEXEC)
- {
- if (script_argv == NULL)
- {
- /* Count the arguments. */
- int argc = 0;
- while (argv[argc++])
- ;
- size_t arglen = (argc + 1) * sizeof (char *);
- if (__libc_use_alloca (alloclen + arglen))
- script_argv = alloca (arglen);
- else
- script_argv = script_argv_malloc = malloc (arglen);
- if (script_argv == NULL)
- {
- /* A possible EACCES error is not as important as
- the ENOMEM. */
- got_eacces = false;
- break;
- }
- scripts_argv (startp, argv, argc, script_argv);
- }
-
- __execve (script_argv[0], script_argv, envp);
- }
-
- switch (errno)
- {
- case EACCES:
- /* Record the we got a `Permission denied' error. If we end
- up finding no executable we can use, we want to diagnose
- that we did find one but were denied access. */
- got_eacces = true;
- case ENOENT:
- case ESTALE:
- case ENOTDIR:
- /* Those errors indicate the file is missing or not executable
- by us, in which case we want to just try the next path
- directory. */
- case ENODEV:
- case ETIMEDOUT:
- /* Some strange filesystems like AFS return even
- stranger error numbers. They cannot reasonably mean
- anything else so ignore those, too. */
- break;
-
- default:
- /* Some other error means we found an executable file, but
- something went wrong executing it; return the error to our
- caller. */
- return -1;
- }
+ case EACCES:
+ /* Record the we got a 'Permission denied' error. If we end
+ up finding no executable we can use, we want to diagnose
+ that we did find one but were denied access. */
+ got_eacces = true;
+ case ENOENT:
+ case ESTALE:
+ case ENOTDIR:
+ /* Those errors indicate the file is missing or not executable
+ by us, in which case we want to just try the next path
+ directory. */
+ case ENODEV:
+ case ETIMEDOUT:
+ /* Some strange filesystems like AFS return even
+ stranger error numbers. They cannot reasonably mean
+ anything else so ignore those, too. */
+ break;
+
+ default:
+ /* Some other error means we found an executable file, but
+ something went wrong executing it; return the error to our
+ caller. */
+ return -1;
}
- while (*p++ != '\0');
-
- /* We tried every element and none of them worked. */
- if (got_eacces)
- /* At least one failure was due to permissions, so report that
- error. */
- __set_errno (EACCES);
- free (script_argv_malloc);
- free (path_malloc);
+ if (*subp++ == '\0')
+ break;
}
- /* Return the error from the last attempt (probably ENOENT). */
+ /* We tried every element and none of them worked. */
+ if (got_eacces)
+ /* At least one failure was due to permissions, so report that
+ error. */
+ __set_errno (EACCES);
+
return -1;
+
}
+
weak_alias (__execvpe, execvpe)
diff --git a/posix/tst-execvp1.c b/posix/tst-execvp1.c
index ecc673d..fe98ce5 100644
--- a/posix/tst-execvp1.c
+++ b/posix/tst-execvp1.c
@@ -3,6 +3,10 @@
#include <stdlib.h>
#include <unistd.h>
+#ifndef EXECVP
+# define EXECVP(__file, __argv) execvp (__file, __argv)
+#endif
+
static int
do_test (void)
{
@@ -19,7 +23,7 @@ do_test (void)
char *argv[] = { (char *) "does-not-exist", NULL };
errno = 0;
- execvp (argv[0], argv);
+ EXECVP (argv[0], argv);
if (errno != ENOENT)
{
diff --git a/posix/tst-execvp2.c b/posix/tst-execvp2.c
index 7e0f5d8..9be8733 100644
--- a/posix/tst-execvp2.c
+++ b/posix/tst-execvp2.c
@@ -14,6 +14,9 @@ static int do_test (void);
#define TEST_FUNCTION do_test ()
#include "../test-skeleton.c"
+#ifndef EXECVP
+# define EXECVP(__file, __argv) execvp (__file, __argv)
+#endif
static char *copy;
@@ -70,7 +73,7 @@ do_test (void)
char *argv[] = { basename (copy), NULL };
errno = 0;
- execvp (argv[0], argv);
+ EXECVP (argv[0], argv);
if (errno != EACCES)
{
diff --git a/posix/tst-execvp3.c b/posix/tst-execvp3.c
index 5ebc879..43f7c34 100644
--- a/posix/tst-execvp3.c
+++ b/posix/tst-execvp3.c
@@ -12,6 +12,9 @@ static int do_test (void);
#include "../test-skeleton.c"
+#ifndef EXECVP
+# define EXECVP(__file, __argv) execvp (__file, __argv)
+#endif
static char *fname;
@@ -35,7 +38,7 @@ do_test (void)
}
char *argv[] = { fname, NULL };
- execvp (basename (fname), argv);
+ EXECVP (basename (fname), argv);
/* If we come here, the execvp call failed. */
return 1;
diff --git a/posix/tst-execvp4.c b/posix/tst-execvp4.c
index 531fab2..116624f 100644
--- a/posix/tst-execvp4.c
+++ b/posix/tst-execvp4.c
@@ -5,6 +5,10 @@
#include <unistd.h>
#include <sys/stat.h>
+#ifndef EXECVP
+# define EXECVP(__file, __argv) execvp (__file, __argv)
+#endif
+
static int
do_test (void)
{
@@ -27,7 +31,7 @@ do_test (void)
unsetenv ("PATH");
char *argv[] = { buf + 9, NULL };
- execvp (argv[0], argv);
+ EXECVP (argv[0], argv);
return 0;
}
diff --git a/posix/tst-execvpe1.c b/posix/tst-execvpe1.c
new file mode 100644
index 0000000..7b386c8
--- /dev/null
+++ b/posix/tst-execvpe1.c
@@ -0,0 +1,20 @@
+/* Check ENOENT failure for execvpe.
+ 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/>. */
+
+#define EXECVP(__file, __argv) execvpe (__file, __argv, NULL)
+#include <posix/tst-execvp1.c>
diff --git a/posix/tst-execvpe2.c b/posix/tst-execvpe2.c
new file mode 100644
index 0000000..3c99fb1
--- /dev/null
+++ b/posix/tst-execvpe2.c
@@ -0,0 +1,20 @@
+/* Check EACCES for execvpe.
+ 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/>. */
+
+#define EXECVP(__file, __argv) execvpe (__file, __argv, NULL)
+#include <posix/tst-execvp2.c>
diff --git a/posix/tst-execvpe3.c b/posix/tst-execvpe3.c
new file mode 100644
index 0000000..8380fd3
--- /dev/null
+++ b/posix/tst-execvpe3.c
@@ -0,0 +1,20 @@
+/* Check script execution without shebang for execvpe.
+ 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/>. */
+
+#define EXECVP(__file, __argv) execvpe (__file, __argv, NULL)
+#include <posix/tst-execvp3.c>
diff --git a/posix/tst-execvpe4.c b/posix/tst-execvpe4.c
new file mode 100644
index 0000000..08fdaf0
--- /dev/null
+++ b/posix/tst-execvpe4.c
@@ -0,0 +1,20 @@
+/* Check unexistent binary for execvpe.
+ 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/>. */
+
+#define EXECVP(__file, __argv) execvpe (__file, __argv, NULL)
+#include <posix/tst-execvp4.c>
diff --git a/posix/tst-execvpe5.c b/posix/tst-execvpe5.c
new file mode 100644
index 0000000..09f0fb0
--- /dev/null
+++ b/posix/tst-execvpe5.c
@@ -0,0 +1,130 @@
+/* General tests for execpve.
+ 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 <errno.h>
+#include <error.h>
+#include <fcntl.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+#include <wait.h>
+
+
+/* Nonzero if the program gets called via `exec'. */
+static int restart;
+
+
+#define CMDLINE_OPTIONS \
+ { "restart", no_argument, &restart, 1 },
+
+/* Prototype for our test function. */
+extern void do_prepare (int argc, char *argv[]);
+extern int do_test (int argc, char *argv[]);
+
+#include "../test-skeleton.c"
+
+#define EXECVPE_KEY "EXECVPE_ENV"
+#define EXECVPE_VALUE "execvpe_test"
+
+
+static int
+handle_restart (void)
+{
+ /* First check if only one variable is passed on execvpe. */
+ int env_count = 0;
+ for (char **e = environ; *e != NULL; ++e)
+ if (++env_count == INT_MAX)
+ error (EXIT_FAILURE, 0, "environment variable number overflow");
+ if (env_count != 1)
+ error (EXIT_FAILURE, 0, "wrong number of environment variables");
+
+ /* Check if the combinarion os "EXECVPE_ENV=execvpe_test" */
+ const char *env = getenv (EXECVPE_KEY);
+ if (env == NULL)
+ error (EXIT_FAILURE, 0, "test environment variable not found");
+
+ if (strncmp (env, EXECVPE_VALUE, sizeof (EXECVPE_VALUE)))
+ error (EXIT_FAILURE, 0, "test environment variable with wrong value");
+
+ return 0;
+}
+
+
+int
+do_test (int argc, char *argv[])
+{
+ pid_t pid;
+ int status;
+
+ /* We must have
+ - one or four parameters left if called initially
+ + path for ld.so optional
+ + "--library-path" optional
+ + the library path optional
+ + the application name
+ */
+
+ if (restart)
+ {
+ if (argc != 1)
+ error (EXIT_FAILURE, 0, "wrong number of arguments (%d)", argc);
+
+ return handle_restart ();
+ }
+
+ if (argc != 2 && argc != 5)
+ error (EXIT_FAILURE, 0, "wrong number of arguments (%d)", argc);
+
+ /* We want to test the `execvpe' function. To do this we restart the
+ program with an additional parameter. */
+ pid = fork ();
+ if (pid == 0)
+ {
+ /* This is the child. Construct the command line. */
+
+ /* We cast here to char* because the test itself does not modify neither
+ the argument nor the environment list. */
+ char *envs[] = { (char*)(EXECVPE_KEY "=" EXECVPE_VALUE), NULL };
+ if (argc == 5)
+ {
+ char *args[] = { argv[1], argv[2], argv[3], argv[4],
+ (char*)"--direct", (char*)"--restart", NULL };
+ execvpe (args[0], args, envs);
+ }
+ else
+ {
+ char *args[] = { argv[1], argv[1],
+ (char*)"--direct", (char*)"--restart", NULL };
+ execvpe (args[0], args, envs);
+ }
+
+ error (EXIT_FAILURE, errno, "cannot exec");
+ }
+ else if (pid == (pid_t) -1)
+ error (EXIT_FAILURE, errno, "cannot fork");
+
+ /* Wait for the child. */
+ if (waitpid (pid, &status, 0) != pid)
+ error (EXIT_FAILURE, errno, "wrong child");
+
+ if (WTERMSIG (status) != 0)
+ error (EXIT_FAILURE, 0, "Child terminated incorrectly");
+ status = WEXITSTATUS (status);
+
+ return status;
+}
diff --git a/posix/tst-execvpe6.c b/posix/tst-execvpe6.c
new file mode 100644
index 0000000..e099a2f
--- /dev/null
+++ b/posix/tst-execvpe6.c
@@ -0,0 +1,82 @@
+/* Check execvpe script argument handling.
+ 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 <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+#include <sys/stat.h>
+#include <sys/param.h>
+
+static char *fname;
+
+static void do_prepare (void);
+#define PREPARE(argc, argv) do_prepare ()
+static int do_test (void);
+#define TEST_FUNCTION do_test ()
+
+#include "../test-skeleton.c"
+
+static void
+do_prepare (void)
+{
+ int fd = create_temp_file ("testscript", &fname);
+ dprintf (fd, "echo foo\n");
+ fchmod (fd, 0700);
+ close (fd);
+}
+
+static int
+do_test (void)
+{
+ if (setenv ("PATH", test_dir, 1) != 0)
+ {
+ puts ("setenv failed");
+ return 1;
+ }
+
+ /* To limit stack allocation for argument construction in case of
+ script without shebang execvpe limits total number of argument
+ to NCARGS. */
+ const size_t max_args = NCARGS + 1;
+ char **args = malloc ((NCARGS + 1) * sizeof (char*));
+ if (args == NULL)
+ {
+ puts ("malloc failed");
+ return 1;
+ }
+
+ args[0] = fname;
+ for (int i=1; i < max_args; ++i)
+ args[i] = strdup ("a");
+ args[max_args] = NULL;
+
+ execvpe (basename (fname), args, NULL);
+
+ for (int i=1; i < max_args; ++i)
+ free (args[i]);
+ free (args);
+
+ if (errno != E2BIG)
+ {
+ printf ("errno = %d (%m), expected E2BIG\n", errno);
+ return 1;
+ }
+
+ return 0;
+}
--
1.9.1
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 1/3] posix: Remove dynamic memory allocation from execl{e,p}
2016-02-19 18:05 [PATCH v2 0/3] posix: Execute file function fixes Adhemerval Zanella
2016-02-19 18:05 ` [PATCH 2/3] posix: execvpe cleanup Adhemerval Zanella
@ 2016-02-19 18:05 ` Adhemerval Zanella
2016-02-20 8:26 ` Mike Frysinger
2016-02-19 18:06 ` [PATCH 3/3] posix: New Linux posix_spawn{p} implementation Adhemerval Zanella
2016-02-19 18:33 ` [PATCH v2 0/3] posix: Execute file function fixes Paul Eggert
3 siblings, 1 reply; 24+ messages in thread
From: Adhemerval Zanella @ 2016-02-19 18:05 UTC (permalink / raw)
To: libc-alpha
GLIBC execl{e,p} implementation might use malloc if the total number of i
arguments exceed initial assumption size (1024). This might lead to
issue in two situations:
1. execl/execle is stated to be async-signal-safe by POSIX [1]. However
if execl is used in a signal handler with a large argument set (that
may call malloc internally) and the resulting call fails, it might
lead malloc in the program in a bad state.
2. If the functions are used in a vfork/clone(VFORK) situation it also
might issue malloc internal bad state.
This patch fixes it by using stack allocation instead. It also fixes
BZ#19534.
Tested on x86_64.
[1] http://pubs.opengroup.org/onlinepubs/9699919799/functions/V2_chap02.html
[BZ #19534]
* posix/execl.c (execl): Remove dynamic memory allocation.
* posix/execle.c (execle): Likewise.
* posix/execlp.c (execlp): Likewise.
---
posix/execl.c | 65 +++++++++++++++++----------------------------------------
posix/execle.c | 66 ++++++++++++++++++----------------------------------------
posix/execlp.c | 64 +++++++++++++++++---------------------------------------
5 files changed, 66 insertions(+), 138 deletions(-)
diff --git a/posix/execl.c b/posix/execl.c
index 102d19d..8b8a324 100644
--- a/posix/execl.c
+++ b/posix/execl.c
@@ -16,58 +16,31 @@
<http://www.gnu.org/licenses/>. */
#include <unistd.h>
+#include <errno.h>
#include <stdarg.h>
-#include <stddef.h>
-#include <stdlib.h>
-#include <string.h>
-
-#include <stackinfo.h>
-
+#include <sys/param.h>
/* Execute PATH with all arguments after PATH until
a NULL pointer and environment from `environ'. */
int
execl (const char *path, const char *arg, ...)
{
-#define INITIAL_ARGV_MAX 1024
- size_t argv_max = INITIAL_ARGV_MAX;
- const char *initial_argv[INITIAL_ARGV_MAX];
- const char **argv = initial_argv;
- va_list args;
-
- argv[0] = arg;
-
- va_start (args, arg);
- unsigned int i = 0;
- while (argv[i++] != NULL)
- {
- if (i == argv_max)
- {
- argv_max *= 2;
- const char **nptr = realloc (argv == initial_argv ? NULL : argv,
- argv_max * sizeof (const char *));
- if (nptr == NULL)
- {
- if (argv != initial_argv)
- free (argv);
- va_end (args);
- return -1;
- }
- if (argv == initial_argv)
- /* We have to copy the already filled-in data ourselves. */
- memcpy (nptr, argv, i * sizeof (const char *));
-
- argv = nptr;
- }
-
- argv[i] = va_arg (args, const char *);
- }
- va_end (args);
-
- int ret = __execve (path, (char *const *) argv, __environ);
- if (argv != initial_argv)
- free (argv);
-
- return ret;
+ int argc;
+ va_list ap;
+ va_start (ap, arg);
+ for (argc = 1; va_arg (ap, const char *); argc++)
+ continue;
+ va_end (ap);
+
+ int i;
+ char *argv[argc+1];
+ va_start (ap, arg);
+ argv[0] = (char*) arg;
+ for (i = 1; i < argc; i++)
+ argv[i] = va_arg (ap, char *);
+ argv[i] = NULL;
+ va_end (ap);
+
+ return __execve (path, argv, __environ);
}
libc_hidden_def (execl)
diff --git a/posix/execle.c b/posix/execle.c
index 8edc03a..1a0c9ee 100644
--- a/posix/execle.c
+++ b/posix/execle.c
@@ -17,57 +17,31 @@
#include <unistd.h>
#include <stdarg.h>
-#include <stddef.h>
-#include <stdlib.h>
-#include <string.h>
-
-#include <stackinfo.h>
+#include <errno.h>
+#include <sys/param.h>
/* Execute PATH with all arguments after PATH until a NULL pointer,
and the argument after that for environment. */
int
execle (const char *path, const char *arg, ...)
{
-#define INITIAL_ARGV_MAX 1024
- size_t argv_max = INITIAL_ARGV_MAX;
- const char *initial_argv[INITIAL_ARGV_MAX];
- const char **argv = initial_argv;
- va_list args;
- argv[0] = arg;
-
- va_start (args, arg);
- unsigned int i = 0;
- while (argv[i++] != NULL)
- {
- if (i == argv_max)
- {
- argv_max *= 2;
- const char **nptr = realloc (argv == initial_argv ? NULL : argv,
- argv_max * sizeof (const char *));
- if (nptr == NULL)
- {
- if (argv != initial_argv)
- free (argv);
- va_end (args);
- return -1;
- }
- if (argv == initial_argv)
- /* We have to copy the already filled-in data ourselves. */
- memcpy (nptr, argv, i * sizeof (const char *));
-
- argv = nptr;
- }
-
- argv[i] = va_arg (args, const char *);
- }
-
- const char *const *envp = va_arg (args, const char *const *);
- va_end (args);
-
- int ret = __execve (path, (char *const *) argv, (char *const *) envp);
- if (argv != initial_argv)
- free (argv);
-
- return ret;
+ int argc;
+ va_list ap;
+ va_start (ap, arg);
+ for (argc = 1; va_arg (ap, const char *); argc++)
+ continue;
+ va_end (ap);
+
+ int i;
+ char *argv[argc+1];
+ char **envp;
+ va_start (ap, arg);
+ argv[0] = (char*) arg;
+ for (i = 1; i < argc; i++)
+ argv[i] = va_arg (ap, char *);
+ envp = va_arg (ap, char **);
+ va_end (ap);
+
+ return __execve (path, argv, envp);
}
libc_hidden_def (execle)
diff --git a/posix/execlp.c b/posix/execlp.c
index 6700994..a0e1859 100644
--- a/posix/execlp.c
+++ b/posix/execlp.c
@@ -17,11 +17,8 @@
#include <unistd.h>
#include <stdarg.h>
-#include <stddef.h>
-#include <stdlib.h>
-#include <string.h>
-
-#include <stackinfo.h>
+#include <errno.h>
+#include <sys/param.h>
/* Execute FILE, searching in the `PATH' environment variable if
it contains no slashes, with all arguments after FILE until a
@@ -29,45 +26,22 @@
int
execlp (const char *file, const char *arg, ...)
{
-#define INITIAL_ARGV_MAX 1024
- size_t argv_max = INITIAL_ARGV_MAX;
- const char *initial_argv[INITIAL_ARGV_MAX];
- const char **argv = initial_argv;
- va_list args;
-
- argv[0] = arg;
-
- va_start (args, arg);
- unsigned int i = 0;
- while (argv[i++] != NULL)
- {
- if (i == argv_max)
- {
- argv_max *= 2;
- const char **nptr = realloc (argv == initial_argv ? NULL : argv,
- argv_max * sizeof (const char *));
- if (nptr == NULL)
- {
- if (argv != initial_argv)
- free (argv);
- va_end (args);
- return -1;
- }
- if (argv == initial_argv)
- /* We have to copy the already filled-in data ourselves. */
- memcpy (nptr, argv, i * sizeof (const char *));
-
- argv = nptr;
- }
-
- argv[i] = va_arg (args, const char *);
- }
- va_end (args);
-
- int ret = execvp (file, (char *const *) argv);
- if (argv != initial_argv)
- free (argv);
-
- return ret;
+ int argc;
+ va_list ap;
+ va_start (ap, arg);
+ for (argc = 1; va_arg (ap, const char *); argc++)
+ continue;
+ va_end (ap);
+
+ int i;
+ char *argv[argc+1];
+ va_start (ap, arg);
+ argv[0] = (char*) arg;
+ for (i = 1; i < argc; i++)
+ argv[i] = va_arg (ap, char *);
+ argv[i] = NULL;
+ va_end (ap);
+
+ return __execvpe (file, argv, __environ);
}
libc_hidden_def (execlp)
--
1.9.1
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH v2 0/3] posix: Execute file function fixes
@ 2016-02-19 18:05 Adhemerval Zanella
2016-02-19 18:05 ` [PATCH 2/3] posix: execvpe cleanup Adhemerval Zanella
` (3 more replies)
0 siblings, 4 replies; 24+ messages in thread
From: Adhemerval Zanella @ 2016-02-19 18:05 UTC (permalink / raw)
To: libc-alpha
This is an update from my previous patchset with fixes based on previous
comments. The differences from previous version are:
* Regarding stack allocation safeness for exec function family I saw no
safe solution. The GCC options '-fstack-check' is hinted as unsafe by
Florian and I am not sure how reliable it is on all the affected
platforms. Also I tend to agree with Rich Felker in the sense libc
has no obligation in make sure the stack allocation is suffice to
fix runtime constraints. No option is added on this patchset to
handle this.
* Also no platform specific asm code with some ABI hacks is added to
avoid the afore-mentioned issue. It only adds code complexity, adds
mantainability, and also adds different platform runtimes behavior and
constraints.
* No CLONE_SETTLS is required for posix_spawn{p} and a comment was added
stating that although parent and child shared the same TLS namespace
there is no concurrent access due the CLONE_VFORK usage.
* Fix an alpha build due its usage of clone2 syscall instead of clone.
--
This patchset add some cleanup and fixes for the exec{l,le,lp,vpe}
general function implementation and fixes long standing bugs for
posix_spawn{p} on Linux. It is basically my previous 2 patchset
for execvpe and posix_spawn{p} along with the execl{e,p} fixes.
For exe{l,le,lp,vpe} function main difference is using stack allocation
instead of dynamic one for argument handling. The main difference from
previous patch iteration is it does not add any memory stack allocation
constraints due:
1. Current GLIBC logic to limit stack allocation through __MAX_ALLOCA_CUTOFF
is arbitrary and does no impose any limit (it does not consider current
stack size neither stack size limit).
2. Memory allocation constraints associated with the functions make
stack allocation the only sane option. All exec function family are
defined to be async-safe, where they can be called either through a
signal handler or in vfork child, and they can not really on dynamic
memory allocation (either through malloc or directly by mmap).
The posix_spawn{p} is a new implementation for Linux which aims to
fix some long-standing bug regarding signal handling. It also
tries to avoid dynamic memory allocation by either relying on the
exec{l,vpe} functions with a dynamic mmap memory region allocate
to use along with direct created child (using clone syscall).
Adhemerval Zanella (3):
posix: Remove dynamic memory allocation from execl{e,p}
posix: execvpe cleanup
posix: New Linux posix_spawn{p} implementation
ChangeLog | 54 +++
include/sched.h | 2 +
include/unistd.h | 1 +
posix/Makefile | 7 +-
posix/execl.c | 65 +---
posix/execle.c | 66 +---
posix/execlp.c | 64 +---
posix/execvpe.c | 238 +++++-------
posix/tst-execvp1.c | 6 +-
posix/tst-execvp2.c | 5 +-
posix/tst-execvp3.c | 5 +-
posix/tst-execvp4.c | 6 +-
posix/tst-execvpe1.c | 20 +
posix/tst-execvpe2.c | 20 +
posix/tst-execvpe3.c | 20 +
posix/tst-execvpe4.c | 20 +
posix/tst-execvpe5.c | 130 +++++++
posix/tst-execvpe6.c | 82 ++++
posix/tst-spawn2.c | 73 ++++
sysdeps/posix/dup.c | 2 +-
sysdeps/unix/sysv/linux/aarch64/clone.S | 1 +
sysdeps/unix/sysv/linux/alpha/clone.S | 1 +
sysdeps/unix/sysv/linux/arm/clone.S | 1 +
sysdeps/unix/sysv/linux/hppa/clone.S | 1 +
sysdeps/unix/sysv/linux/i386/clone.S | 1 +
sysdeps/unix/sysv/linux/ia64/clone2.S | 2 +
sysdeps/unix/sysv/linux/m68k/clone.S | 1 +
sysdeps/unix/sysv/linux/microblaze/clone.S | 1 +
sysdeps/unix/sysv/linux/mips/clone.S | 1 +
sysdeps/unix/sysv/linux/nios2/clone.S | 1 +
sysdeps/unix/sysv/linux/nptl-signals.h | 10 +
sysdeps/unix/sysv/linux/powerpc/powerpc32/clone.S | 1 +
sysdeps/unix/sysv/linux/powerpc/powerpc64/clone.S | 1 +
sysdeps/unix/sysv/linux/s390/s390-32/clone.S | 2 +
sysdeps/unix/sysv/linux/s390/s390-64/clone.S | 2 +
sysdeps/unix/sysv/linux/sh/clone.S | 1 +
sysdeps/unix/sysv/linux/sparc/sparc32/clone.S | 1 +
sysdeps/unix/sysv/linux/sparc/sparc64/clone.S | 1 +
sysdeps/unix/sysv/linux/spawni.c | 433 ++++++++++++++++++++++
sysdeps/unix/sysv/linux/tile/clone.S | 1 +
sysdeps/unix/sysv/linux/x86_64/clone.S | 1 +
41 files changed, 1065 insertions(+), 286 deletions(-)
create mode 100644 posix/tst-execvpe1.c
create mode 100644 posix/tst-execvpe2.c
create mode 100644 posix/tst-execvpe3.c
create mode 100644 posix/tst-execvpe4.c
create mode 100644 posix/tst-execvpe5.c
create mode 100644 posix/tst-execvpe6.c
create mode 100644 posix/tst-spawn2.c
create mode 100644 sysdeps/unix/sysv/linux/spawni.c
--
1.9.1
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 3/3] posix: New Linux posix_spawn{p} implementation
2016-02-19 18:05 [PATCH v2 0/3] posix: Execute file function fixes Adhemerval Zanella
2016-02-19 18:05 ` [PATCH 2/3] posix: execvpe cleanup Adhemerval Zanella
2016-02-19 18:05 ` [PATCH 1/3] posix: Remove dynamic memory allocation from execl{e,p} Adhemerval Zanella
@ 2016-02-19 18:06 ` Adhemerval Zanella
2016-02-19 18:33 ` [PATCH v2 0/3] posix: Execute file function fixes Paul Eggert
3 siblings, 0 replies; 24+ messages in thread
From: Adhemerval Zanella @ 2016-02-19 18:06 UTC (permalink / raw)
To: libc-alpha
This patch implements a new posix_spawn{p} implementation for Linux. The main
difference is it uses the clone syscall directly with CLONE_VM and CLONE_VFORK
flags and a direct allocated stack. The new stack and start function solves
most the vfork limitation (possible parent clobber due stack spilling). The
remaning issue are related to signal handling:
1. That no signal handlers must run in child context, to avoid corrupt
parent's state.
2. Child must synchronize with parent to enforce stack deallocation and
to possible return execv issues.
The first one is solved by blocking all signals in child, even NPTL-internal
ones (SIGCANCEL and SIGSETXID). The second issue is done by a stack allocation
in parent and a synchronization with using a pipe or waitpid (in case or error).
The pipe has the advantage of allowing the child signal an exec error (checked
with new tst-spawn2 test).
There is an inherent race condition in pipe2 usage for architectures that do not
support the syscall directly. In such cases the a pipe plus fctnl is used
instead and it may lead to file descriptor leak in parent (as decribed by fcntl
documentation).
The child process stack is allocate with a mmap with MAP_STACK flag using
default architecture stack size. Although it is slower than use a stack buffer
from parent, it allows some slack for the compatibility code to run scripts
with no shebang (which may use a buffer with size depending of argument list
count).
Performance should be similar to the vfork default posix implementation and
way faster than fork path (vfork on mostly linux ports are basically
clone with CLONE_VM plus CLONE_VFORK). The only difference is the syscalls
required for the stack allocation/deallocation.
It fixes BZ#10354, BZ#14750, and BZ#18433.
Tested on i386, x86_64, powerpc64le, and aarch64.
[BZ #14750]
[BZ #10354]
[BZ #18433]
* include/sched.h (__clone): Add hidden prototype.
(__clone2): Likewise.
* include/unistd.h (__dup): Likewise.
* posix/Makefile (tests): Add tst-spawn2.
* posix/tst-spawn2.c: New file.
* sysdeps/posix/dup.c (__dup): Add hidden definition.
* sysdeps/unix/sysv/linux/aarch64/clone.S (__clone): Likewise.
* sysdeps/unix/sysv/linux/alpha/clone.S (__clone): Likewise.
* sysdeps/unix/sysv/linux/arm/clone.S (__clone): Likewise.
* sysdeps/unix/sysv/linux/hppa/clone.S (__clone): Likewise.
* sysdeps/unix/sysv/linux/i386/clone.S (__clone): Likewise.
* sysdeps/unix/sysv/linux/ia64/clone2.S (__clone): Likewise.
* sysdeps/unix/sysv/linux/m68k/clone.S (__clone): Likewise.
* sysdeps/unix/sysv/linux/microblaze/clone.S (__clone): Likewise.
* sysdeps/unix/sysv/linux/mips/clone.S (__clone): Likewise.
* sysdeps/unix/sysv/linux/nios2/clone.S (__clone): Likewise.
* sysdeps/unix/sysv/linux/powerpc/powerpc32/clone.S (__clone):
Likewise.
* sysdeps/unix/sysv/linux/powerpc/powerpc64/clone.S (__clone):
Likewise.
* sysdeps/unix/sysv/linux/s390/s390-32/clone.S (__clone): Likewise.
* sysdeps/unix/sysv/linux/s390/s390-64/clone.S (__clone): Likewise.
* sysdeps/unix/sysv/linux/sh/clone.S (__clone): Likewise.
* sysdeps/unix/sysv/linux/sparc/sparc32/clone.S (__clone): Likewise.
* sysdeps/unix/sysv/linux/sparc/sparc64/clone.S (__clone): Likewise.
* sysdeps/unix/sysv/linux/tile/clone.S (__clone): Likewise.
* sysdeps/unix/sysv/linux/x86_64/clone.S (__clone): Likewise.
* sysdeps/unix/sysv/linux/nptl-signals.h
(____nptl_is_internal_signal): New function.
* sysdeps/unix/sysv/linux/spawni.c: New file.
---
include/sched.h | 2 +
include/unistd.h | 1 +
posix/Makefile | 2 +-
posix/tst-spawn2.c | 73 ++++
sysdeps/posix/dup.c | 2 +-
sysdeps/unix/sysv/linux/aarch64/clone.S | 1 +
sysdeps/unix/sysv/linux/alpha/clone.S | 1 +
sysdeps/unix/sysv/linux/arm/clone.S | 1 +
sysdeps/unix/sysv/linux/hppa/clone.S | 1 +
sysdeps/unix/sysv/linux/i386/clone.S | 1 +
sysdeps/unix/sysv/linux/ia64/clone2.S | 2 +
sysdeps/unix/sysv/linux/m68k/clone.S | 1 +
sysdeps/unix/sysv/linux/microblaze/clone.S | 1 +
sysdeps/unix/sysv/linux/mips/clone.S | 1 +
sysdeps/unix/sysv/linux/nios2/clone.S | 1 +
sysdeps/unix/sysv/linux/nptl-signals.h | 10 +
sysdeps/unix/sysv/linux/powerpc/powerpc32/clone.S | 1 +
sysdeps/unix/sysv/linux/powerpc/powerpc64/clone.S | 1 +
sysdeps/unix/sysv/linux/s390/s390-32/clone.S | 2 +
sysdeps/unix/sysv/linux/s390/s390-64/clone.S | 2 +
sysdeps/unix/sysv/linux/sh/clone.S | 1 +
sysdeps/unix/sysv/linux/sparc/sparc32/clone.S | 1 +
sysdeps/unix/sysv/linux/sparc/sparc64/clone.S | 1 +
sysdeps/unix/sysv/linux/spawni.c | 433 ++++++++++++++++++++++
sysdeps/unix/sysv/linux/tile/clone.S | 1 +
sysdeps/unix/sysv/linux/x86_64/clone.S | 1 +
27 files changed, 577 insertions(+), 2 deletions(-)
create mode 100644 posix/tst-spawn2.c
create mode 100644 sysdeps/unix/sysv/linux/spawni.c
diff --git a/include/sched.h b/include/sched.h
index 4f59397..b4d7406 100644
--- a/include/sched.h
+++ b/include/sched.h
@@ -19,7 +19,9 @@ extern int __sched_rr_get_interval (__pid_t __pid, struct timespec *__t);
/* These are Linux specific. */
extern int __clone (int (*__fn) (void *__arg), void *__child_stack,
int __flags, void *__arg, ...);
+libc_hidden_proto (__clone)
extern int __clone2 (int (*__fn) (void *__arg), void *__child_stack_base,
size_t __child_stack_size, int __flags, void *__arg, ...);
+libc_hidden_proto (__clone2)
#endif
#endif
diff --git a/include/unistd.h b/include/unistd.h
index 5152f64..d2802b2 100644
--- a/include/unistd.h
+++ b/include/unistd.h
@@ -81,6 +81,7 @@ char *__canonicalize_directory_name_internal (const char *__thisdir,
size_t __size) attribute_hidden;
extern int __dup (int __fd);
+libc_hidden_proto (__dup)
extern int __dup2 (int __fd, int __fd2);
libc_hidden_proto (__dup2)
extern int __dup3 (int __fd, int __fd2, int flags);
diff --git a/posix/Makefile b/posix/Makefile
index a2aabcc..a38a3a6 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
+tests += wordexp-test tst-exec tst-spawn tst-spawn2
endif
tests-static = tst-exec-static tst-spawn-static
tests += $(tests-static)
diff --git a/posix/tst-spawn2.c b/posix/tst-spawn2.c
new file mode 100644
index 0000000..41bfde8
--- /dev/null
+++ b/posix/tst-spawn2.c
@@ -0,0 +1,73 @@
+/* Further tests for spawn in case of invalid binary paths.
+ 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 <spawn.h>
+#include <error.h>
+#include <errno.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+#include <sys/wait.h>
+
+#include <stdio.h>
+
+int
+posix_spawn_test (void)
+{
+ /* Check if posix_spawn correctly returns an error and an invalid pid
+ by trying to spawn an invalid binary. */
+
+ const char *program = "/path/to/invalid/binary";
+ char * const args[] = { 0 };
+ pid_t pid = -1;
+
+ int ret = posix_spawn (&pid, program, 0, 0, args, environ);
+ if (ret != ENOENT)
+ error (EXIT_FAILURE, errno, "posix_spawn");
+
+ /* POSIX states the value returned on pid variable in case of an error
+ is not specified. GLIBC will update the value iff the child
+ execution is successful. */
+ if (pid != -1)
+ error (EXIT_FAILURE, errno, "posix_spawn returned pid != -1");
+
+ /* Check if no child is actually created. */
+ ret = waitpid (-1, NULL, 0);
+ if (ret != -1 || errno != ECHILD)
+ error (EXIT_FAILURE, errno, "waitpid");
+
+ /* Same as before, but with posix_spawnp. */
+ char *args2[] = { (char*) program, 0 };
+
+ ret = posix_spawnp (&pid, args2[0], 0, 0, args2, environ);
+ if (ret != ENOENT)
+ error (EXIT_FAILURE, errno, "posix_spawnp");
+
+ if (pid != -1)
+ error (EXIT_FAILURE, errno, "posix_spawnp returned pid != -1");
+
+ ret = waitpid (-1, NULL, 0);
+ if (ret != -1 || errno != ECHILD)
+ error (EXIT_FAILURE, errno, "waitpid");
+
+ return 0;
+}
+
+#define TEST_FUNCTION posix_spawn_test ()
+#include "../test-skeleton.c"
+
diff --git a/sysdeps/posix/dup.c b/sysdeps/posix/dup.c
index 9525e76..abf3ff5 100644
--- a/sysdeps/posix/dup.c
+++ b/sysdeps/posix/dup.c
@@ -26,5 +26,5 @@ __dup (int fd)
{
return fcntl (fd, F_DUPFD, 0);
}
-
+libc_hidden_def (__dup)
weak_alias (__dup, dup)
diff --git a/sysdeps/unix/sysv/linux/aarch64/clone.S b/sysdeps/unix/sysv/linux/aarch64/clone.S
index 596fb9c..8f3ab40 100644
--- a/sysdeps/unix/sysv/linux/aarch64/clone.S
+++ b/sysdeps/unix/sysv/linux/aarch64/clone.S
@@ -93,4 +93,5 @@ thread_start:
cfi_endproc
.size thread_start, .-thread_start
+libc_hidden_def (__clone)
weak_alias (__clone, clone)
diff --git a/sysdeps/unix/sysv/linux/alpha/clone.S b/sysdeps/unix/sysv/linux/alpha/clone.S
index ec9c06d..556aa63 100644
--- a/sysdeps/unix/sysv/linux/alpha/clone.S
+++ b/sysdeps/unix/sysv/linux/alpha/clone.S
@@ -137,4 +137,5 @@ thread_start:
cfi_endproc
.end thread_start
+libc_hidden_def (__clone)
weak_alias (__clone, clone)
diff --git a/sysdeps/unix/sysv/linux/arm/clone.S b/sysdeps/unix/sysv/linux/arm/clone.S
index 460a8f5..c103123 100644
--- a/sysdeps/unix/sysv/linux/arm/clone.S
+++ b/sysdeps/unix/sysv/linux/arm/clone.S
@@ -93,4 +93,5 @@ PSEUDO_END (__clone)
.fnend
+libc_hidden_def (__clone)
weak_alias (__clone, clone)
diff --git a/sysdeps/unix/sysv/linux/hppa/clone.S b/sysdeps/unix/sysv/linux/hppa/clone.S
index e80fd8d..0a18137 100644
--- a/sysdeps/unix/sysv/linux/hppa/clone.S
+++ b/sysdeps/unix/sysv/linux/hppa/clone.S
@@ -175,4 +175,5 @@ ENTRY(__clone)
PSEUDO_END(__clone)
+libc_hidden_def (__clone)
weak_alias (__clone, clone)
diff --git a/sysdeps/unix/sysv/linux/i386/clone.S b/sysdeps/unix/sysv/linux/i386/clone.S
index 7d818c1..ef447d1 100644
--- a/sysdeps/unix/sysv/linux/i386/clone.S
+++ b/sysdeps/unix/sysv/linux/i386/clone.S
@@ -139,4 +139,5 @@ L(nomoregetpid):
cfi_startproc
PSEUDO_END (__clone)
+libc_hidden_def (__clone)
weak_alias (__clone, clone)
diff --git a/sysdeps/unix/sysv/linux/ia64/clone2.S b/sysdeps/unix/sysv/linux/ia64/clone2.S
index 9b257b3..fc046eb 100644
--- a/sysdeps/unix/sysv/linux/ia64/clone2.S
+++ b/sysdeps/unix/sysv/linux/ia64/clone2.S
@@ -96,6 +96,8 @@ ENTRY(__clone2)
ret /* Not reached. */
PSEUDO_END(__clone2)
+libc_hidden_def (__clone2)
+
/* For now we leave __clone undefined. This is unlikely to be a */
/* problem, since at least the i386 __clone in glibc always failed */
/* with a 0 sp (eventhough the kernel explicitly handled it). */
diff --git a/sysdeps/unix/sysv/linux/m68k/clone.S b/sysdeps/unix/sysv/linux/m68k/clone.S
index 8b40df2..33474cc 100644
--- a/sysdeps/unix/sysv/linux/m68k/clone.S
+++ b/sysdeps/unix/sysv/linux/m68k/clone.S
@@ -127,4 +127,5 @@ donepid:
cfi_startproc
PSEUDO_END (__clone)
+libc_hidden_def (__clone)
weak_alias (__clone, clone)
diff --git a/sysdeps/unix/sysv/linux/microblaze/clone.S b/sysdeps/unix/sysv/linux/microblaze/clone.S
index 035d88b..cbc95ce 100644
--- a/sysdeps/unix/sysv/linux/microblaze/clone.S
+++ b/sysdeps/unix/sysv/linux/microblaze/clone.S
@@ -69,4 +69,5 @@ L(thread_start):
nop
PSEUDO_END(__clone)
+libc_hidden_def (__clone)
weak_alias (__clone,clone)
diff --git a/sysdeps/unix/sysv/linux/mips/clone.S b/sysdeps/unix/sysv/linux/mips/clone.S
index 755e8cc..feb8250 100644
--- a/sysdeps/unix/sysv/linux/mips/clone.S
+++ b/sysdeps/unix/sysv/linux/mips/clone.S
@@ -166,4 +166,5 @@ L(gotpid):
END(__thread_start)
+libc_hidden_def (__clone)
weak_alias (__clone, clone)
diff --git a/sysdeps/unix/sysv/linux/nios2/clone.S b/sysdeps/unix/sysv/linux/nios2/clone.S
index 4da5c19..e4d3970 100644
--- a/sysdeps/unix/sysv/linux/nios2/clone.S
+++ b/sysdeps/unix/sysv/linux/nios2/clone.S
@@ -104,4 +104,5 @@ thread_start:
cfi_startproc
PSEUDO_END (__clone)
+libc_hidden_def (__clone)
weak_alias (__clone, clone)
diff --git a/sysdeps/unix/sysv/linux/nptl-signals.h b/sysdeps/unix/sysv/linux/nptl-signals.h
index 7560a21..01f34c2 100644
--- a/sysdeps/unix/sysv/linux/nptl-signals.h
+++ b/sysdeps/unix/sysv/linux/nptl-signals.h
@@ -16,6 +16,8 @@
License along with the GNU C Library; if not, see
<http://www.gnu.org/licenses/>. */
+#include <signal.h>
+
/* The signal used for asynchronous cancelation. */
#define SIGCANCEL __SIGRTMIN
@@ -29,5 +31,13 @@
/* Signal used to implement the setuid et.al. functions. */
#define SIGSETXID (__SIGRTMIN + 1)
+
+/* Return is sig is used internally. */
+static inline int
+__nptl_is_internal_signal (int sig)
+{
+ return (sig == SIGCANCEL) || (sig == SIGTIMER) || (sig == SIGSETXID);
+}
+
/* Used to communicate with signal handler. */
extern struct xid_command *__xidcmd attribute_hidden;
diff --git a/sysdeps/unix/sysv/linux/powerpc/powerpc32/clone.S b/sysdeps/unix/sysv/linux/powerpc/powerpc32/clone.S
index 28948ea..eb973db 100644
--- a/sysdeps/unix/sysv/linux/powerpc/powerpc32/clone.S
+++ b/sysdeps/unix/sysv/linux/powerpc/powerpc32/clone.S
@@ -108,4 +108,5 @@ L(badargs):
cfi_startproc
END (__clone)
+libc_hidden_def (__clone)
weak_alias (__clone, clone)
diff --git a/sysdeps/unix/sysv/linux/powerpc/powerpc64/clone.S b/sysdeps/unix/sysv/linux/powerpc/powerpc64/clone.S
index c8c6de8..959fdb7 100644
--- a/sysdeps/unix/sysv/linux/powerpc/powerpc64/clone.S
+++ b/sysdeps/unix/sysv/linux/powerpc/powerpc64/clone.S
@@ -126,4 +126,5 @@ L(parent):
END (__clone)
+libc_hidden_def (__clone)
weak_alias (__clone, clone)
diff --git a/sysdeps/unix/sysv/linux/s390/s390-32/clone.S b/sysdeps/unix/sysv/linux/s390/s390-32/clone.S
index cb2afbb..f774e90 100644
--- a/sysdeps/unix/sysv/linux/s390/s390-32/clone.S
+++ b/sysdeps/unix/sysv/linux/s390/s390-32/clone.S
@@ -70,4 +70,6 @@ thread_start:
xc 0(4,%r15),0(%r15)
basr %r14,%r1 /* jump to fn */
DO_CALL (exit, 1)
+
+libc_hidden_def (__clone)
weak_alias (__clone, clone)
diff --git a/sysdeps/unix/sysv/linux/s390/s390-64/clone.S b/sysdeps/unix/sysv/linux/s390/s390-64/clone.S
index eddab35..928a881 100644
--- a/sysdeps/unix/sysv/linux/s390/s390-64/clone.S
+++ b/sysdeps/unix/sysv/linux/s390/s390-64/clone.S
@@ -73,4 +73,6 @@ thread_start:
xc 0(8,%r15),0(%r15)
basr %r14,%r1 /* jump to fn */
DO_CALL (exit, 1)
+
+libc_hidden_def (__clone)
weak_alias (__clone, clone)
diff --git a/sysdeps/unix/sysv/linux/sh/clone.S b/sysdeps/unix/sysv/linux/sh/clone.S
index 53cc08b..a73dd7d 100644
--- a/sysdeps/unix/sysv/linux/sh/clone.S
+++ b/sysdeps/unix/sysv/linux/sh/clone.S
@@ -123,4 +123,5 @@ ENTRY(__clone)
.word TID - TLS_PRE_TCB_SIZE
PSEUDO_END (__clone)
+libc_hidden_def (__clone)
weak_alias (__clone, clone)
diff --git a/sysdeps/unix/sysv/linux/sparc/sparc32/clone.S b/sysdeps/unix/sysv/linux/sparc/sparc32/clone.S
index 6f41f0f..68f8202 100644
--- a/sysdeps/unix/sysv/linux/sparc/sparc32/clone.S
+++ b/sysdeps/unix/sysv/linux/sparc/sparc32/clone.S
@@ -100,4 +100,5 @@ __thread_start:
.size __thread_start, .-__thread_start
+libc_hidden_def (__clone)
weak_alias (__clone, clone)
diff --git a/sysdeps/unix/sysv/linux/sparc/sparc64/clone.S b/sysdeps/unix/sysv/linux/sparc/sparc64/clone.S
index e2d3914..cecffa7 100644
--- a/sysdeps/unix/sysv/linux/sparc/sparc64/clone.S
+++ b/sysdeps/unix/sysv/linux/sparc/sparc64/clone.S
@@ -96,4 +96,5 @@ __thread_start:
.size __thread_start, .-__thread_start
+libc_hidden_def (__clone)
weak_alias (__clone, clone)
diff --git a/sysdeps/unix/sysv/linux/spawni.c b/sysdeps/unix/sysv/linux/spawni.c
new file mode 100644
index 0000000..8bdce2a
--- /dev/null
+++ b/sysdeps/unix/sysv/linux/spawni.c
@@ -0,0 +1,433 @@
+/* POSIX spawn interface. Linux version.
+ 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 <spawn.h>
+#include <fcntl.h>
+#include <paths.h>
+#include <string.h>
+#include <sys/resource.h>
+#include <sys/wait.h>
+#include <sys/param.h>
+#include <sys/mman.h>
+#include <not-cancel.h>
+#include <local-setxid.h>
+#include <shlib-compat.h>
+#include <nptl/pthreadP.h>
+#include <dl-sysdep.h>
+#include <ldsodefs.h>
+#include "spawn_int.h"
+
+/* The Linux implementation of posix_spawn{p} uses the clone syscall directly
+ with CLONE_VM and CLONE_VFORK flags and an allocated stack. The new stack
+ and start function solves most the vfork limitation (possible parent
+ clobber due stack spilling). The remaning issue are:
+
+ 1. That no signal handlers must run in child context, to avoid corrupt
+ parent's state.
+ 2. The parent must ensure child's stack freeing.
+ 3. Child must synchronize with parent to enforce 2. and to possible
+ return execv issues.
+
+ The first issue is solved by blocking all signals in child, even the
+ NPTL-internal ones (SIGCANCEL and SIGSETXID). The second and third issue
+ is done by a stack allocation in parent and a synchronization with using
+ a pipe or waitpid (in case or error). The pipe has the advantage of
+ allowing the child the signal an exec error. */
+
+
+/* The Unix standard contains a long explanation of the way to signal
+ an error after the fork() was successful. Since no new wait status
+ was wanted there is no way to signal an error using one of the
+ available methods. The committee chose to signal an error by a
+ normal program exit with the exit code 127. */
+#define SPAWN_ERROR 127
+
+/* We need to block both SIGCANCEL and SIGSETXID. */
+#define SIGALL_SET \
+ ((__sigset_t) { .__val = {[0 ... _SIGSET_NWORDS-1 ] = -1 } })
+
+#ifdef __ia64__
+# define CLONE(__fn, __stack, __stacksize, __flags, __args) \
+ __clone2 (__fn, __stack, __stacksize, __flags, __args, 0, 0, 0)
+#else
+# define CLONE(__fn, __stack, __stacksize, __flags, __args) \
+ __clone (__fn, __stack, __flags, __args)
+#endif
+
+#if _STACK_GROWS_DOWN
+# define STACK(__stack, __stack_size) (__stack + __stack_size)
+#elif _STACK_GROWS_UP
+# define STACK(__stack, __stack_size) (__stack)
+#endif
+
+/* Issues a 'pipe2' when the architecture supports or a 'pipe' plus O_CLOEXEC
+ otherwise. When pipe2 syscall is not available the pipe plus fcntl have a
+ inherent race condition (pipe2 is the right solution to avoid it). */
+
+#ifdef __ASSUME_PIPE2
+# define __have_pipe2 1
+#endif
+
+static int
+__do_pipe2_cloexec (int pipe_fds[2])
+{
+ /* Try pipe2 even if __ASSUME_PIPE2 is not define and returning an error
+ iff the call returns ENOSYS. */
+ int r = -1;
+ if (__have_pipe2 >= 0)
+ {
+ r = __pipe2 (pipe_fds, O_CLOEXEC);
+#ifndef __ASSUME_PIPE2
+ if (__have_pipe2 == 0)
+ __have_pipe2 = r != -1 || errno != ENOSYS ? 1 : -1;
+#endif
+
+ if (__have_pipe2 > 0)
+ return r;
+ }
+ if (__have_pipe2 < 0)
+ if (__pipe (pipe_fds) < 0)
+ return -1;
+
+ /* Then apply FD_CLOCEXEC if it is supported in the pipe call case. */
+ if (__have_pipe2 < 0)
+ {
+ if ((r = __fcntl (pipe_fds[0], F_SETFD, FD_CLOEXEC)) == -1
+ || (r = __fcntl (pipe_fds[1], F_SETFD, FD_CLOEXEC)) == -1)
+ {
+ close_not_cancel (pipe_fds[0]);
+ close_not_cancel (pipe_fds[1]);
+ return r;
+ }
+ }
+
+ return r;
+}
+
+struct posix_spawn_args
+{
+ int pipe[2];
+ sigset_t oldmask;
+ const char *file;
+ int (*exec) (const char *, char *const *, char *const *);
+ const posix_spawn_file_actions_t *fa;
+ const posix_spawnattr_t *restrict attr;
+ char *const *argv;
+ char *const *envp;
+ int xflags;
+};
+
+/* Older version requires that shell script without shebang definition
+ to be called explicity using /bin/sh (_PATH_BSHELL). */
+static void
+maybe_script_execute (struct posix_spawn_args *args)
+{
+ if (SHLIB_COMPAT (libc, GLIBC_2_2, GLIBC_2_15)
+ && (args->xflags & SPAWN_XFLAGS_TRY_SHELL) && errno == ENOEXEC)
+ {
+ char *const *argv = args->argv;
+ int argc = 0;
+ while (argv[argc++] != NULL)
+ continue;
+
+ /* Construct an argument list for the shell. */
+ char *new_argv[argc];
+ new_argv[0] = (char *) _PATH_BSHELL;
+ new_argv[1] = (char *) args->file;
+ while (argc > 1)
+ {
+ new_argv[argc] = argv[argc - 1];
+ --argc;
+ }
+
+ /* Execute the shell. */
+ args->exec (new_argv[0], new_argv, args->envp);
+ }
+}
+
+/* Function used in the clone call to setup the signals mask, posix_spawn
+ attributes, and file actions. It run on its own stack (provided by the
+ posix_spawn call). */
+static int
+__spawni_child (void *arguments)
+{
+ struct posix_spawn_args *args = arguments;
+ const posix_spawnattr_t *restrict attr = args->attr;
+ const posix_spawn_file_actions_t *file_actions = args->fa;
+ int p = args->pipe[1];
+ int ret;
+
+ close_not_cancel (args->pipe[0]);
+
+ /* The child must ensure that no signal handler are enabled because it shared
+ memory with parent, so the signal disposition must be either SIG_DFL or
+ SIG_IGN. It does by iterating over all signals and although it could
+ possibly be more optimized (by tracking which signal potentially have a
+ signal handler), it might requires system specific solutions (since the
+ sigset_t data type can be very different on different architectures). */
+ struct sigaction sa;
+ memset (&sa, '\0', sizeof (sa));
+
+ sigset_t hset;
+ __sigprocmask (SIG_BLOCK, 0, &hset);
+ for (int sig = 1; sig < _NSIG; ++sig)
+ {
+ if ((attr->__flags & POSIX_SPAWN_SETSIGDEF)
+ && sigismember (&attr->__sd, sig))
+ {
+ sa.sa_handler = SIG_DFL;
+ }
+ else if (sigismember (&hset, sig))
+ {
+ if (__nptl_is_internal_signal (sig))
+ sa.sa_handler = SIG_IGN;
+ else
+ {
+ __libc_sigaction (sig, 0, &sa);
+ if (sa.sa_handler == SIG_IGN)
+ continue;
+ sa.sa_handler = SIG_DFL;
+ }
+ }
+ else
+ continue;
+
+ __libc_sigaction (sig, &sa, 0);
+ }
+
+#ifdef _POSIX_PRIORITY_SCHEDULING
+ /* Set the scheduling algorithm and parameters. */
+ if ((attr->__flags & (POSIX_SPAWN_SETSCHEDPARAM | POSIX_SPAWN_SETSCHEDULER))
+ == POSIX_SPAWN_SETSCHEDPARAM)
+ {
+ if ((ret = __sched_setparam (0, &attr->__sp)) == -1)
+ goto fail;
+ }
+ else if ((attr->__flags & POSIX_SPAWN_SETSCHEDULER) != 0)
+ {
+ if ((ret = __sched_setscheduler (0, attr->__policy, &attr->__sp)) == -1)
+ goto fail;
+ }
+#endif
+
+ /* Set the process group ID. */
+ if ((attr->__flags & POSIX_SPAWN_SETPGROUP) != 0
+ && (ret = __setpgid (0, attr->__pgrp)) != 0)
+ goto fail;
+
+ /* Set the effective user and group IDs. */
+ if ((attr->__flags & POSIX_SPAWN_RESETIDS) != 0
+ && ((ret = local_seteuid (__getuid ())) != 0
+ || (ret = local_setegid (__getgid ())) != 0))
+ goto fail;
+
+ /* Execute the file actions. */
+ if (file_actions != 0)
+ {
+ int cnt;
+ struct rlimit64 fdlimit;
+ bool have_fdlimit = false;
+
+ for (cnt = 0; cnt < file_actions->__used; ++cnt)
+ {
+ struct __spawn_action *action = &file_actions->__actions[cnt];
+
+ /* Dup the pipe fd onto an unoccupied one to avoid any file
+ operation to clobber it. */
+ if ((action->action.close_action.fd == p)
+ || (action->action.open_action.fd == p)
+ || (action->action.dup2_action.fd == p))
+ {
+ if ((ret = __dup (p)) < 0)
+ goto fail;
+ p = ret;
+ }
+
+ switch (action->tag)
+ {
+ case spawn_do_close:
+ if ((ret =
+ close_not_cancel (action->action.close_action.fd)) != 0)
+ {
+ if (!have_fdlimit)
+ {
+ __getrlimit64 (RLIMIT_NOFILE, &fdlimit);
+ have_fdlimit = true;
+ }
+
+ /* Signal errors only for file descriptors out of range. */
+ if (action->action.close_action.fd < 0
+ || action->action.close_action.fd >= fdlimit.rlim_cur)
+ goto fail;
+ }
+ break;
+
+ case spawn_do_open:
+ {
+ ret = open_not_cancel (action->action.open_action.path,
+ action->action.
+ open_action.oflag | O_LARGEFILE,
+ action->action.open_action.mode);
+
+ if (ret == -1)
+ goto fail;
+
+ int new_fd = ret;
+
+ /* Make sure the desired file descriptor is used. */
+ if (ret != action->action.open_action.fd)
+ {
+ if ((ret = __dup2 (new_fd, action->action.open_action.fd))
+ != action->action.open_action.fd)
+ goto fail;
+
+ if ((ret = close_not_cancel (new_fd)) != 0)
+ goto fail;
+ }
+ }
+ break;
+
+ case spawn_do_dup2:
+ if ((ret = __dup2 (action->action.dup2_action.fd,
+ action->action.dup2_action.newfd))
+ != action->action.dup2_action.newfd)
+ goto fail;
+ break;
+ }
+ }
+ }
+
+ /* Set the initial signal mask of the child if POSIX_SPAWN_SETSIGMASK
+ is set, otherwise restore the previous one. */
+ __sigprocmask (SIG_SETMASK, (attr->__flags & POSIX_SPAWN_SETSIGMASK)
+ ? &attr->__ss : &args->oldmask, 0);
+
+ args->exec (args->file, args->argv, args->envp);
+
+ /* This is compatibility function required to enable posix_spawn run
+ script without shebang definition for older posix_spawn versions
+ (2.15). */
+ maybe_script_execute (args);
+
+ ret = -errno;
+
+fail:
+ /* Since sizeof errno < PIPE_BUF, the write is atomic. */
+ ret = -ret;
+ if (ret)
+ while (write_not_cancel (p, &ret, sizeof ret) < 0)
+ continue;
+ exit (SPAWN_ERROR);
+}
+
+/* Spawn a new process executing PATH with the attributes describes in *ATTRP.
+ Before running the process perform the actions described in FILE-ACTIONS. */
+static int
+__spawnix (pid_t * pid, const char *file,
+ const posix_spawn_file_actions_t * file_actions,
+ const posix_spawnattr_t * attrp, char *const argv[],
+ char *const envp[], int xflags,
+ int (*exec) (const char *, char *const *, char *const *))
+{
+ pid_t new_pid;
+ struct posix_spawn_args args;
+ int ec;
+
+ if (__do_pipe2_cloexec (args.pipe))
+ return errno;
+
+ /* Allocates a child stack with sufficient size to handle a large argument
+ set in both the 'maybe_script_execute' compatibility code and on the
+ execvpe (posix_spawnp) where it might allocate stack size when path
+ does not contain a slash. */
+ const int prot = (PROT_READ | PROT_WRITE
+ | ((GL (dl_stack_flags) & PF_X) ? PROT_EXEC : 0));
+
+ const int stack_size = ARCH_STACK_DEFAULT_SIZE;
+ void *stack = __mmap (NULL, stack_size, prot,
+ MAP_PRIVATE | MAP_ANONYMOUS | MAP_STACK, -1, 0);
+ if (__glibc_unlikely (stack == MAP_FAILED))
+ {
+ close_not_cancel (args.pipe[0]);
+ close_not_cancel (args.pipe[1]);
+ return errno;
+ }
+
+ /* Disable asynchronous cancellation. */
+ int cs = LIBC_CANCEL_ASYNC ();
+
+ args.file = file;
+ args.exec = exec;
+ args.fa = file_actions;
+ args.attr = attrp ? attrp : &(const posix_spawnattr_t) { 0 };
+ args.argv = argv;
+ args.envp = envp;
+ args.xflags = xflags;
+
+ __sigprocmask (SIG_BLOCK, &SIGALL_SET, &args.oldmask);
+
+ /* The clone flags used will create a new child that will run in the same
+ memory space (CLONE_VM) and the execution of calling thread will be
+ suspend until the child calls execve or _exit. These condition as
+ signal below either by pipe write (_exit with SPAWN_ERROR) or
+ a successful execve.
+ Also since the calling thread execution will be suspend, there is not
+ need for CLONE_SETTLS. Although parent and child share the same TLS
+ namespace, there will be no concurrent access for TLS variables (errno
+ for instance). */
+ new_pid = CLONE (__spawni_child, STACK (stack, stack_size), stack_size,
+ CLONE_VM | CLONE_VFORK | SIGCHLD, &args);
+
+ close_not_cancel (args.pipe[1]);
+
+ if (new_pid > 0)
+ {
+ if (__read (args.pipe[0], &ec, sizeof ec) != sizeof ec)
+ ec = 0;
+ else
+ __waitpid (new_pid, NULL, 0);
+ }
+ else
+ ec = -new_pid;
+
+ __munmap (stack, stack_size);
+
+ close_not_cancel (args.pipe[0]);
+
+ if (!ec && new_pid)
+ *pid = new_pid;
+
+ __sigprocmask (SIG_SETMASK, &args.oldmask, 0);
+
+ LIBC_CANCEL_RESET (cs);
+
+ return ec;
+}
+
+/* Spawn a new process executing PATH with the attributes describes in *ATTRP.
+ Before running the process perform the actions described in FILE-ACTIONS. */
+int
+__spawni (pid_t * pid, const char *file,
+ const posix_spawn_file_actions_t * acts,
+ const posix_spawnattr_t * attrp, char *const argv[],
+ char *const envp[], int xflags)
+{
+ if (xflags & SPAWN_XFLAGS_USE_PATH)
+ return __spawnix (pid, file, acts, attrp, argv, envp, xflags, __execvpe);
+ return __spawnix (pid, file, acts, attrp, argv, envp, xflags, __execve);
+}
diff --git a/sysdeps/unix/sysv/linux/tile/clone.S b/sysdeps/unix/sysv/linux/tile/clone.S
index 02fe3a8..a300eb5 100644
--- a/sysdeps/unix/sysv/linux/tile/clone.S
+++ b/sysdeps/unix/sysv/linux/tile/clone.S
@@ -219,4 +219,5 @@ ENTRY (__clone)
}
PSEUDO_END (__clone)
+libc_hidden_def (__clone)
weak_alias (__clone, clone)
diff --git a/sysdeps/unix/sysv/linux/x86_64/clone.S b/sysdeps/unix/sysv/linux/x86_64/clone.S
index 382568a..1a50957 100644
--- a/sysdeps/unix/sysv/linux/x86_64/clone.S
+++ b/sysdeps/unix/sysv/linux/x86_64/clone.S
@@ -115,4 +115,5 @@ L(thread_start):
cfi_startproc;
PSEUDO_END (__clone)
+libc_hidden_def (__clone)
weak_alias (__clone, clone)
--
1.9.1
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2 0/3] posix: Execute file function fixes
2016-02-19 18:05 [PATCH v2 0/3] posix: Execute file function fixes Adhemerval Zanella
` (2 preceding siblings ...)
2016-02-19 18:06 ` [PATCH 3/3] posix: New Linux posix_spawn{p} implementation Adhemerval Zanella
@ 2016-02-19 18:33 ` Paul Eggert
2016-02-19 19:14 ` Adhemerval Zanella
2016-02-19 23:12 ` Joseph Myers
3 siblings, 2 replies; 24+ messages in thread
From: Paul Eggert @ 2016-02-19 18:33 UTC (permalink / raw)
To: Adhemerval Zanella, libc-alpha
On 02/19/2016 10:05 AM, Adhemerval Zanella wrote:
> * Regarding stack allocation safeness for exec function family I saw no
> safe solution.
This is a significant regression from the current behavior. We need a
better solution. Otherwise, I fear that it will be too easy for
attackers to exploit stack-overflow vulnerabilities by attempting to
execute commands with many arguments.
> libc has no obligation in make sure the stack allocation is suffice to
> fix runtime constraints.
Is this really true? Then why does libc have __libc_use_alloca? Why not
dispense with __libc_use_alloca and have libc impose no limits on stack
allocation?
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2 0/3] posix: Execute file function fixes
2016-02-19 18:33 ` [PATCH v2 0/3] posix: Execute file function fixes Paul Eggert
@ 2016-02-19 19:14 ` Adhemerval Zanella
2016-02-19 19:58 ` Paul Eggert
2016-02-19 23:12 ` Joseph Myers
1 sibling, 1 reply; 24+ messages in thread
From: Adhemerval Zanella @ 2016-02-19 19:14 UTC (permalink / raw)
To: Paul Eggert, libc-alpha
On 19-02-2016 16:33, Paul Eggert wrote:
> On 02/19/2016 10:05 AM, Adhemerval Zanella wrote:
>> * Regarding stack allocation safeness for exec function family I saw no
>> safe solution.
>
> This is a significant regression from the current behavior. We need a better solution. Otherwise, I fear that it will be too easy for attackers to exploit stack-overflow vulnerabilities by attempting to execute commands with many arguments.
>
I do not agree it is a regression since it fixes two important issues:
exec async-signal-safeness and vfork usage. Also the vector of attack
might be limited, since for calling these function will issue a lot of
stack will usage for argument passing.
As I said, current dynamic memory allocation usage is plainly wrong and
lead to the two mentioned issues. We can go to arch-specific implementation
that abuse the ABI with, but again I think they are hacks and just add
more maintainability burden and runtime differences among the platforms.
A possible solution is add thread specific scratch buffer, but again this
is not async-safe.
>> libc has no obligation in make sure the stack allocation is suffice to
>> fix runtime constraints.
>
> Is this really true? Then why does libc have __libc_use_alloca? Why not dispense with __libc_use_alloca and have libc impose no limits on stack allocation?
The current '__libc_use_alloca' is a fragile and arbitrary stack control flow
and since it does not track total stack usage against runtime imposed limit
it does not add anything related to security. Currently as is I see no
compelling reason to continue using it.
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2 0/3] posix: Execute file function fixes
2016-02-19 19:14 ` Adhemerval Zanella
@ 2016-02-19 19:58 ` Paul Eggert
2016-02-19 20:19 ` Adhemerval Zanella
0 siblings, 1 reply; 24+ messages in thread
From: Paul Eggert @ 2016-02-19 19:58 UTC (permalink / raw)
To: Adhemerval Zanella, libc-alpha
On 02/19/2016 11:14 AM, Adhemerval Zanella wrote:
> I do not agree it is a regression since it fixes two important issues:
> exec async-signal-safeness and vfork usage.
Even if a change makes improvements in some ways, the change can still
be a regression in other ways. The proposed change plainly has such
regressions.
> the vector of attack might be limited
Agreed. Still, the vector is there.
> The current '__libc_use_alloca' is a fragile and arbitrary stack
> control flow and since it does not track total stack usage against
> runtime imposed limit it does not add anything related to security.
Certainly __libc_use_alloca is an imperfect mechanism. However, if code
touches buffers before going past them, __libc_use_alloca results in
reliable stack overflow protection in the common case where
sufficiently-large guard pages are at the top of the stack. So it's an
exaggeration to say that __libc_use_alloca adds nothing to security.
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2 0/3] posix: Execute file function fixes
2016-02-19 19:58 ` Paul Eggert
@ 2016-02-19 20:19 ` Adhemerval Zanella
2016-02-19 23:13 ` Paul Eggert
0 siblings, 1 reply; 24+ messages in thread
From: Adhemerval Zanella @ 2016-02-19 20:19 UTC (permalink / raw)
To: Paul Eggert; +Cc: libc-alpha
> Em 19 de fev de 2016, às 17:58, Paul Eggert <eggert@cs.ucla.edu> escreveu:
>
>> On 02/19/2016 11:14 AM, Adhemerval Zanella wrote:
>> I do not agree it is a regression since it fixes two important issues:
>> exec async-signal-safeness and vfork usage.
>
> Even if a change makes improvements in some ways, the change can still be a regression in other ways. The proposed change plainly has such regressions.
>
>> the vector of attack might be limited
>
> Agreed. Still, the vector is there.
And I am open to suggestions on how to harden the code against attacks, but I do see it as blocker to prevent since current implementations have serious limitations.
And it is a regression only if you consider dynamic allocation an alternative, which is not the case.
>
>> The current '__libc_use_alloca' is a fragile and arbitrary stack control flow and since it does not track total stack usage against runtime imposed limit it does not add anything related to security.
>
> Certainly __libc_use_alloca is an imperfect mechanism. However, if code touches buffers before going past them, __libc_use_alloca results in reliable stack overflow protection in the common case where sufficiently-large guard pages are at the top of the stack. So it's an exaggeration to say that __libc_use_alloca adds nothing to security.
The problem it is not realible, it adds code complexity in buffer handling, it does track current stack usage, the limit is arbitrary. It is very limited scope implementation of fstack-check.
A better strategy would be set either hard limits on stack usage in design phase or to focus on enable GLIBC to use a better stack protection mechanism (preferable through compiler assistance).
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2 0/3] posix: Execute file function fixes
2016-02-19 18:33 ` [PATCH v2 0/3] posix: Execute file function fixes Paul Eggert
2016-02-19 19:14 ` Adhemerval Zanella
@ 2016-02-19 23:12 ` Joseph Myers
2016-02-19 23:26 ` Paul Eggert
1 sibling, 1 reply; 24+ messages in thread
From: Joseph Myers @ 2016-02-19 23:12 UTC (permalink / raw)
To: Paul Eggert; +Cc: Adhemerval Zanella, libc-alpha
On Fri, 19 Feb 2016, Paul Eggert wrote:
> On 02/19/2016 10:05 AM, Adhemerval Zanella wrote:
> > * Regarding stack allocation safeness for exec function family I saw no
> > safe solution.
>
> This is a significant regression from the current behavior. We need a better
> solution. Otherwise, I fear that it will be too easy for attackers to exploit
> stack-overflow vulnerabilities by attempting to execute commands with many
> arguments.
This is a case where, as noted in
<https://sourceware.org/ml/libc-alpha/2016-02/msg00044.html>, the stack
usage is proportional to the number of arguments passed by the caller -
that is, it's something determined statically at compile time, not under
the control of an attacker.
While alloca of an amount proportional to the number or size of arguments
passed is not ideal, I don't think it's a vulnerability the way any
unbounded alloca of an amount not proportional to stack space already used
is.
--
Joseph S. Myers
joseph@codesourcery.com
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2 0/3] posix: Execute file function fixes
2016-02-19 20:19 ` Adhemerval Zanella
@ 2016-02-19 23:13 ` Paul Eggert
2016-02-20 10:37 ` Adhemerval Zanella
0 siblings, 1 reply; 24+ messages in thread
From: Paul Eggert @ 2016-02-19 23:13 UTC (permalink / raw)
To: Adhemerval Zanella; +Cc: libc-alpha
[-- Attachment #1: Type: text/plain, Size: 1280 bytes --]
On 02/19/2016 12:19 PM, Adhemerval Zanella wrote:
> And it is a regression only if you consider dynamic allocation an alternative,
It's a regression in that it would break some applications that
currently work. An application that is simple (no signal handling,
single-threaded, etc.) can invoke programs with many arguments now, but
the same application won't work if that patch is installed.
Come to think of it, execlp, posix_spawn, and posix_spawnp can all use
malloc, since POSIX does not require them to be async-signal-safe. So
you can go back to using malloc for their implementations, when they are
given large argument vectors. The only conformance problem here will be
execl and execle.
For these two, I suggest a machine-dependent implementation, with the
default being something along the lines of the attached (this is totally
untested, I'm just trying to give the gist).
> A better strategy would be set either hard limits on stack usage in design phase or to focus on enable GLIBC to use a better stack protection mechanism (preferable through compiler assistance).
The latter sounds like a good idea, but is a bigger project and I doubt
whether we'd want to wait for that project to be finished before fixing
the exec-family problems in question.
[-- Attachment #2: execl.c --]
[-- Type: text/plain, Size: 1571 bytes --]
#include <unistd.h>
#include <stdarg.h>
#include <stddef.h>
#include <string.h>
#include <stdlib.h>
int
execl (const char *file, const char *arg, ...)
{
enum { N = 1024 };
char *argv[N];
va_list args;
int argc = 0;
argv[0] = (char *) arg;
va_start (args, arg);
while (argv[argc++] != NULL && argc < N)
argv[argc] = va_arg (args, char *);
va_end (args);
if (argv[argc - 1] == NULL)
return __execve (file, argv, __environ);
/* The argument vector does not fit into ARGV. Find out where in
the stack the caller put the arguments, store the initial
arguments there (as they are often cached in registers), and use
that part of the stack as an argument vector. If the exec fails,
restore the temporarily-trashed stack. This is not guaranteed to
work on all platforms, but it should be portable to most, and the
exceptions need to have machine-specific replacements. */
char **parg = (char **) &arg;
for (int i = N / 2; ; i++)
{
int j;
for (j = 0; parg[i + j] == argv[N / 2 + j]; j++)
if (j == N / 2)
{
char **reused_argv = parg + i - N / 2;
size_t half_size = N / 2 * sizeof *argv;
/* Save the part of the stack that will be temporarily trashed. */
memcpy (argv + N / 2, reused_argv, half_size);
/* Temporarily trash the stack. */
memcpy (reused_argv, argv, half_size);
__execve (file, reused_argv, __environ);
/* Restore the temporarily-trashed stack. */
memcpy (reused_argv, argv + N / 2, half_size);
return -1;
}
}
}
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2 0/3] posix: Execute file function fixes
2016-02-19 23:12 ` Joseph Myers
@ 2016-02-19 23:26 ` Paul Eggert
0 siblings, 0 replies; 24+ messages in thread
From: Paul Eggert @ 2016-02-19 23:26 UTC (permalink / raw)
To: Joseph Myers; +Cc: Adhemerval Zanella, libc-alpha
On 02/19/2016 03:11 PM, Joseph Myers wrote:
> This is a case where, as noted in
> <https://sourceware.org/ml/libc-alpha/2016-02/msg00044.html>, the stack
> usage is proportional to the number of arguments passed by the caller -
> that is, it's something determined statically at compile time, not under
> the control of an attacker.
True, and that removes most of my objection to the change to execl,
execle, and execlp. This limitation should be documented, though. (I
still like the idea of reusing the stack and removing the limitation,
but that's lower priority.)
However, the objection remains for posix_spawn and posix_spawnp, where
the number of arguments is not determined statically. Luckily these two
functions do not need to be async-signal-safe, so they can call malloc
when there are too many arguments.
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 1/3] posix: Remove dynamic memory allocation from execl{e,p}
2016-02-19 18:05 ` [PATCH 1/3] posix: Remove dynamic memory allocation from execl{e,p} Adhemerval Zanella
@ 2016-02-20 8:26 ` Mike Frysinger
0 siblings, 0 replies; 24+ messages in thread
From: Mike Frysinger @ 2016-02-20 8:26 UTC (permalink / raw)
To: Adhemerval Zanella; +Cc: libc-alpha
[-- Attachment #1: Type: text/plain, Size: 540 bytes --]
On 19 Feb 2016 16:05, Adhemerval Zanella wrote:
> int
> execl (const char *path, const char *arg, ...)
> {
> ...
> + char *argv[argc+1];
style nit: needs spaces around the +. comes up in this patch more than once.
> + argv[0] = (char*) arg;
space before the *
> int
> execle (const char *path, const char *arg, ...)
> {
> ...
> + va_start (ap, arg);
> + argv[0] = (char*) arg;
> + for (i = 1; i < argc; i++)
> + argv[i] = va_arg (ap, char *);
> + envp = va_arg (ap, char **);
is argv missing a NULL terminator ?
-mike
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 2/3] posix: execvpe cleanup
2016-02-19 18:05 ` [PATCH 2/3] posix: execvpe cleanup Adhemerval Zanella
@ 2016-02-20 8:32 ` Mike Frysinger
0 siblings, 0 replies; 24+ messages in thread
From: Mike Frysinger @ 2016-02-20 8:32 UTC (permalink / raw)
To: Adhemerval Zanella; +Cc: libc-alpha
[-- Attachment #1: Type: text/plain, Size: 1306 bytes --]
On 19 Feb 2016 16:05, Adhemerval Zanella wrote:
> + if (!*subp)
i think GNU style says this should be:
if (*subp == NULL)
> + /* And the file to execute. */
> + memcpy (buffer + (subp - p) + (subp > p), file, file_len + 1);
adding (subp > p) as 0/1 is weird. maybe put !! in front of it so it's
clear what you meant ?
> + case EACCES:
> + /* Record the we got a 'Permission denied' error. If we end
> + up finding no executable we can use, we want to diagnose
> + that we did find one but were denied access. */
bad indentation -- should start w/tabs
> + case ETIMEDOUT:
> + /* Some strange filesystems like AFS return even
> + stranger error numbers. They cannot reasonably mean
> + anything else so ignore those, too. */
same here
> +# define EXECVP(__file, __argv) execvp (__file, __argv)
do these macros really need __ prefixes ?
> + error (EXIT_FAILURE, 0, "environment variable number overflow");
error writes to stderr. our style is for tests to use printf w/stdout.
> + (char*)"--direct", (char*)"--restart", NULL };
space before the *. comes up multiple times in the patch.
> + for (int i=1; i < max_args; ++i)
should be spaces around the =. comes up multiple times in the patch.
-mike
[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2 0/3] posix: Execute file function fixes
2016-02-19 23:13 ` Paul Eggert
@ 2016-02-20 10:37 ` Adhemerval Zanella
2016-02-20 18:25 ` Paul Eggert
0 siblings, 1 reply; 24+ messages in thread
From: Adhemerval Zanella @ 2016-02-20 10:37 UTC (permalink / raw)
To: Paul Eggert; +Cc: libc-alpha
> Em 19 de fev de 2016, às 21:13, Paul Eggert <eggert@cs.ucla.edu> escreveu:
>
>> On 02/19/2016 12:19 PM, Adhemerval Zanella wrote:
>> And it is a regression only if you consider dynamic allocation an alternative,
>
> It's a regression in that it would break some applications that currently work. An application that is simple (no signal handling, single-threaded, etc.) can invoke programs with many arguments now, but the same application won't work if that patch is installed.
>
It is debatable since it is using a non-conforming implementation do to so.
> Come to think of it, execlp, posix_spawn, and posix_spawnp can all use malloc, since POSIX does not require them to be async-signal-safe. So you can go back to using malloc for their implementations, when they are given large argument vectors. The only conformance problem here will be execl and execle.
Execlp can not because since it can be used in following vfork. Posix-spawn can, although In match I noted a mmap area with map_stack to provide a better strategy since it may use a large allocated area only in certain cases.
>
> For these two, I suggest a machine-dependent implementation, with the default being something along the lines of the attached (this is totally untested, I'm just trying to give the gist).
>
>> A better strategy would be set either hard limits on stack usage in design phase or to focus on enable GLIBC to use a better stack protection mechanism (preferable through compiler assistance).
>
> The latter sounds like a good idea, but is a bigger project and I doubt whether we'd want to wait for that project to be finished before fixing the exec-family problems in question.
> <execl.c>
I agree, but I also do not see a point in rely on faulty stack allocation mechanisms.
Anyway I see with Joseph comments the only missing point is total mmap area of posix_spawn which sets a hard limit for old shell argument handling and posix_spawnp. We can either document these limit or add a logic to calculate total argument list and mmap an area to accommodate all of it.
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH v2 0/3] posix: Execute file function fixes
2016-02-20 10:37 ` Adhemerval Zanella
@ 2016-02-20 18:25 ` Paul Eggert
0 siblings, 0 replies; 24+ messages in thread
From: Paul Eggert @ 2016-02-20 18:25 UTC (permalink / raw)
To: Adhemerval Zanella; +Cc: libc-alpha
Adhemerval Zanella wrote:
> Execlp can not because since it can be used in following vfork.
I guess that's OK. Please document that execlp has the same limitations that
execl and execle do, in this area.
> the only missing point is total mmap area of posix_spawn which sets a hard limit for old shell argument handling and posix_spawnp. We can either document these limit or add a logic to calculate total argument list and mmap an area to accommodate all of it.
It's better to not impose hard limits of our own.
One other hard limit is that argc cannot exceed INT_MAX; posix_spawn* should
ensure that the argc it computes doesn't go past that.
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 2/3] posix: execvpe cleanup
2016-02-29 18:34 ` [PATCH 2/3] posix: execvpe cleanup Adhemerval Zanella
@ 2016-02-29 19:34 ` Paul Eggert
0 siblings, 0 replies; 24+ messages in thread
From: Paul Eggert @ 2016-02-29 19:34 UTC (permalink / raw)
To: Adhemerval Zanella, libc-alpha
On 02/29/2016 10:33 AM, Adhemerval Zanella wrote:
> + if ((argc+1) == INT_MAX)
This is clearer as:
if (argc == INT_MAX - 1)
Spaces around binary "+" and/or "-", please, and avoid redundant parens.
To some extent I've reviewed the code so often that I'm burned out as a
reviewer, but it looks OK to me other than these minor glitches.
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 2/3] posix: execvpe cleanup
2016-02-29 18:33 [PATCH v5 0/3] posix: Execute file function fixes Adhemerval Zanella
@ 2016-02-29 18:34 ` Adhemerval Zanella
2016-02-29 19:34 ` Paul Eggert
0 siblings, 1 reply; 24+ messages in thread
From: Adhemerval Zanella @ 2016-02-29 18:34 UTC (permalink / raw)
To: libc-alpha
This patch removes all the dynamic allocation on execvpe code and
instead use direct stack allocation. This is QoI approach to make
it possible use in scenarios where memory is shared with parent
(vfork or clone with CLONE_VM).
For default process spawn (script file without a shebang), stack
allocation is bounded by NAME_MAX plus PATH_MAX plus 1. Large
file arguments returns an error (ENAMETOOLONG). This differs than
current GLIBC pratice in general, but it used to limit stack
allocation for large inputs. Also, path in PATH environment variable
larger than PATH_MAX are ignored.
The shell direct execution exeception, where execve returns ENOEXEC,
might requires a large stack allocation due large input argument list.
Tested on i686, x86_64, powerpc64le, and aarch64.
* posix/execvpe.c (__execvpe): Remove dynamic allocation.
* posix/Makefile (tests): Add tst-execvpe{1,2,3,4,5,6}.
* posix/tst-execvp1.c (do_test): Use a macro to call execvp.
* posix/tst-execvp2.c (do_test): Likewise.
* posix/tst-execvp3.c (do_test): Likewise.
* posix/tst-execvp4.c (do_test): Likewise.
* posix/tst-execvpe1.c: New file.
* posix/tst-execvpe2.c: Likewise.
* posix/tst-execvpe3.c: Likewise.
* posix/tst-execvpe4.c: Likewise.
* posix/tst-execvpe5.c: Likewise.
* posix/tst-execvpe6.c: Likewise.
---
posix/Makefile | 3 +
posix/execvpe.c | 253 +++++++++++++++++++++------------------------------
posix/tst-execvp1.c | 6 +-
posix/tst-execvp2.c | 5 +-
posix/tst-execvp3.c | 5 +-
posix/tst-execvp4.c | 6 +-
posix/tst-execvpe1.c | 20 ++++
posix/tst-execvpe2.c | 20 ++++
posix/tst-execvpe3.c | 20 ++++
posix/tst-execvpe4.c | 20 ++++
posix/tst-execvpe5.c | 157 ++++++++++++++++++++++++++++++++
posix/tst-execvpe6.c | 150 ++++++++++++++++++++++++++++++
13 files changed, 527 insertions(+), 151 deletions(-)
create mode 100644 posix/tst-execvpe1.c
create mode 100644 posix/tst-execvpe2.c
create mode 100644 posix/tst-execvpe3.c
create mode 100644 posix/tst-execvpe4.c
create mode 100644 posix/tst-execvpe5.c
create mode 100644 posix/tst-execvpe6.c
diff --git a/posix/Makefile b/posix/Makefile
index 4e90a95..7ec110e 100644
--- a/posix/Makefile
+++ b/posix/Makefile
@@ -82,6 +82,8 @@ tests := tstgetopt testfnm runtests runptests \
tst-execv1 tst-execv2 tst-execl1 tst-execl2 \
tst-execve1 tst-execve2 tst-execle1 tst-execle2 \
tst-execvp3 tst-execvp4 tst-rfc3484 tst-rfc3484-2 \
+ tst-execvpe1 tst-execvpe2 tst-execvpe3 tst-execvpe4 \
+ tst-execvpe5 tst-execvpe6 \
tst-rfc3484-3 \
tst-getaddrinfo3 tst-fnmatch2 tst-cpucount tst-cpuset \
bug-getopt1 bug-getopt2 bug-getopt3 bug-getopt4 \
@@ -229,6 +231,7 @@ tstgetopt-ARGS = -a -b -cfoobar --required foobar --optional=bazbug \
tst-exec-ARGS = -- $(host-test-program-cmd)
tst-exec-static-ARGS = $(tst-exec-ARGS)
+tst-execvpe5-ARGS = -- $(host-test-program-cmd)
tst-spawn-ARGS = -- $(host-test-program-cmd)
tst-spawn-static-ARGS = $(tst-spawn-ARGS)
tst-dir-ARGS = `pwd` `cd $(common-objdir)/$(subdir); pwd` `cd $(common-objdir); pwd` $(objpfx)tst-dir
diff --git a/posix/execvpe.c b/posix/execvpe.c
index 61697a7..6b490b1 100644
--- a/posix/execvpe.c
+++ b/posix/execvpe.c
@@ -15,7 +15,7 @@
License along with the GNU C Library; if not, see
<http://www.gnu.org/licenses/>. */
-#include <alloca.h>
+#include <stdio.h>
#include <unistd.h>
#include <stdarg.h>
#include <stdbool.h>
@@ -23,22 +23,40 @@
#include <string.h>
#include <errno.h>
#include <paths.h>
+#include <confstr.h>
+#include <sys/param.h>
+#ifndef PATH_MAX
+# ifdef MAXPATHLEN
+# define PATH_MAX MAXPATHLEN
+# else
+# define PATH_MAX 1024
+# endif
+#endif
/* The file is accessible but it is not an executable file. Invoke
the shell to interpret it as a script. */
static void
-internal_function
-scripts_argv (const char *file, char *const argv[], int argc, char **new_argv)
+maybe_script_execute (const char *file, char *const argv[], char *const envp[])
{
+ ptrdiff_t argc = 0;
+ while (argv[argc++] != NULL)
+ {
+ if ((argc+1) == INT_MAX)
+ {
+ errno = E2BIG;
+ return;
+ }
+ }
+
/* Construct an argument list for the shell. */
+ char *new_argv[argc + 1];
new_argv[0] = (char *) _PATH_BSHELL;
new_argv[1] = (char *) file;
- while (argc > 1)
- {
- new_argv[argc] = argv[argc - 1];
- --argc;
- }
+ memcpy (new_argv + 2, argv + 1, argc * sizeof(char *));
+
+ /* Execute the shell. */
+ __execve (new_argv[0], new_argv, envp);
}
@@ -47,170 +65,111 @@ scripts_argv (const char *file, char *const argv[], int argc, char **new_argv)
int
__execvpe (const char *file, char *const argv[], char *const envp[])
{
+ /* We check the simple case first. */
if (*file == '\0')
{
- /* We check the simple case first. */
__set_errno (ENOENT);
return -1;
}
+ /* Don't search when it contains a slash. */
if (strchr (file, '/') != NULL)
{
- /* Don't search when it contains a slash. */
__execve (file, argv, envp);
if (errno == ENOEXEC)
- {
- /* Count the arguments. */
- int argc = 0;
- while (argv[argc++])
- ;
- size_t len = (argc + 1) * sizeof (char *);
- char **script_argv;
- void *ptr = NULL;
- if (__libc_use_alloca (len))
- script_argv = alloca (len);
- else
- script_argv = ptr = malloc (len);
-
- if (script_argv != NULL)
- {
- scripts_argv (file, argv, argc, script_argv);
- __execve (script_argv[0], script_argv, envp);
-
- free (ptr);
- }
- }
+ maybe_script_execute (file, argv, envp);
+
+ return -1;
}
- else
+
+ const char *path = getenv ("PATH");
+ if (!path)
+ path = CS_PATH;
+ /* Although GLIBC does not enforce NAME_MAX, we set it as the maximum
+ size to avoid unbounded stack allocation. Same applies for
+ PATH_MAX. */
+ size_t file_len = __strnlen (file, NAME_MAX + 1);
+ size_t path_len = __strnlen (path, PATH_MAX - 1) + 1;
+
+ if ((file_len > NAME_MAX)
+ || !__libc_alloca_cutoff (path_len + file_len + 1))
{
- size_t pathlen;
- size_t alloclen = 0;
- char *path = getenv ("PATH");
- if (path == NULL)
- {
- pathlen = confstr (_CS_PATH, (char *) NULL, 0);
- alloclen = pathlen + 1;
- }
- else
- pathlen = strlen (path);
+ errno = ENAMETOOLONG;
+ return -1;
+ }
- size_t len = strlen (file) + 1;
- alloclen += pathlen + len + 1;
+ const char *subp;
+ bool got_eacces = false;
+ char buffer[path_len + file_len + 1];
+ for (const char *p = path; ; p = subp)
+ {
+ subp = __strchrnul (p, ':');
- char *name;
- char *path_malloc = NULL;
- if (__libc_use_alloca (alloclen))
- name = alloca (alloclen);
- else
+ /* PATH is larger than PATH_MAX and thus potentially larger than
+ the stack allocation. */
+ if (subp - p >= path_len)
{
- path_malloc = name = malloc (alloclen);
- if (name == NULL)
- return -1;
+ /* If there is only one path, bail out. */
+ if (*subp == '\0')
+ break;
+ /* Otherwise skip to next one. */
+ continue;
}
- if (path == NULL)
- {
- /* There is no `PATH' in the environment.
- The default search path is the current directory
- followed by the path `confstr' returns for `_CS_PATH'. */
- path = name + pathlen + len + 1;
- path[0] = ':';
- (void) confstr (_CS_PATH, path + 1, pathlen);
- }
+ /* Use the current path entry, plus a '/' if nonempty, plus the file to
+ execute. */
+ char *pend = mempcpy (buffer, p, subp - p);
+ *pend = '/';
+ memcpy (pend + (p < subp), file, file_len + 1);
- /* Copy the file name at the top. */
- name = (char *) memcpy (name + pathlen + 1, file, len);
- /* And add the slash. */
- *--name = '/';
+ __execve (buffer, argv, envp);
- char **script_argv = NULL;
- void *script_argv_malloc = NULL;
- bool got_eacces = false;
- char *p = path;
- do
+ if (errno == ENOEXEC)
+ /* This has O(P*C) behavior, where P is the length of the path and C
+ is the argument count. A better strategy would be allocate the
+ substitute argv and reuse it each time through the loop (so it
+ behaves as O(P+C) instead. */
+ maybe_script_execute (buffer, argv, envp);
+
+ switch (errno)
{
- char *startp;
-
- path = p;
- p = __strchrnul (path, ':');
-
- if (p == path)
- /* Two adjacent colons, or a colon at the beginning or the end
- of `PATH' means to search the current directory. */
- startp = name + 1;
- else
- startp = (char *) memcpy (name - (p - path), path, p - path);
-
- /* Try to execute this name. If it works, execve will not return. */
- __execve (startp, argv, envp);
-
- if (errno == ENOEXEC)
- {
- if (script_argv == NULL)
- {
- /* Count the arguments. */
- int argc = 0;
- while (argv[argc++])
- ;
- size_t arglen = (argc + 1) * sizeof (char *);
- if (__libc_use_alloca (alloclen + arglen))
- script_argv = alloca (arglen);
- else
- script_argv = script_argv_malloc = malloc (arglen);
- if (script_argv == NULL)
- {
- /* A possible EACCES error is not as important as
- the ENOMEM. */
- got_eacces = false;
- break;
- }
- scripts_argv (startp, argv, argc, script_argv);
- }
-
- __execve (script_argv[0], script_argv, envp);
- }
-
- switch (errno)
- {
- case EACCES:
- /* Record the we got a `Permission denied' error. If we end
- up finding no executable we can use, we want to diagnose
- that we did find one but were denied access. */
- got_eacces = true;
- case ENOENT:
- case ESTALE:
- case ENOTDIR:
- /* Those errors indicate the file is missing or not executable
- by us, in which case we want to just try the next path
- directory. */
- case ENODEV:
- case ETIMEDOUT:
- /* Some strange filesystems like AFS return even
- stranger error numbers. They cannot reasonably mean
- anything else so ignore those, too. */
- break;
-
- default:
- /* Some other error means we found an executable file, but
- something went wrong executing it; return the error to our
- caller. */
- return -1;
- }
+ case EACCES:
+ /* Record that we got a 'Permission denied' error. If we end
+ up finding no executable we can use, we want to diagnose
+ that we did find one but were denied access. */
+ got_eacces = true;
+ case ENOENT:
+ case ESTALE:
+ case ENOTDIR:
+ /* Those errors indicate the file is missing or not executable
+ by us, in which case we want to just try the next path
+ directory. */
+ case ENODEV:
+ case ETIMEDOUT:
+ /* Some strange filesystems like AFS return even
+ stranger error numbers. They cannot reasonably mean
+ anything else so ignore those, too. */
+ break;
+
+ default:
+ /* Some other error means we found an executable file, but
+ something went wrong executing it; return the error to our
+ caller. */
+ return -1;
}
- while (*p++ != '\0');
- /* We tried every element and none of them worked. */
- if (got_eacces)
- /* At least one failure was due to permissions, so report that
- error. */
- __set_errno (EACCES);
-
- free (script_argv_malloc);
- free (path_malloc);
+ if (*subp++ == '\0')
+ break;
}
- /* Return the error from the last attempt (probably ENOENT). */
+ /* We tried every element and none of them worked. */
+ if (got_eacces)
+ /* At least one failure was due to permissions, so report that
+ error. */
+ __set_errno (EACCES);
+
return -1;
}
+
weak_alias (__execvpe, execvpe)
diff --git a/posix/tst-execvp1.c b/posix/tst-execvp1.c
index ecc673d..8b71848 100644
--- a/posix/tst-execvp1.c
+++ b/posix/tst-execvp1.c
@@ -3,6 +3,10 @@
#include <stdlib.h>
#include <unistd.h>
+#ifndef EXECVP
+# define EXECVP(file, argv) execvp (file, argv)
+#endif
+
static int
do_test (void)
{
@@ -19,7 +23,7 @@ do_test (void)
char *argv[] = { (char *) "does-not-exist", NULL };
errno = 0;
- execvp (argv[0], argv);
+ EXECVP (argv[0], argv);
if (errno != ENOENT)
{
diff --git a/posix/tst-execvp2.c b/posix/tst-execvp2.c
index 7e0f5d8..440dfab 100644
--- a/posix/tst-execvp2.c
+++ b/posix/tst-execvp2.c
@@ -14,6 +14,9 @@ static int do_test (void);
#define TEST_FUNCTION do_test ()
#include "../test-skeleton.c"
+#ifndef EXECVP
+# define EXECVP(file, argv) execvp (file, argv)
+#endif
static char *copy;
@@ -70,7 +73,7 @@ do_test (void)
char *argv[] = { basename (copy), NULL };
errno = 0;
- execvp (argv[0], argv);
+ EXECVP (argv[0], argv);
if (errno != EACCES)
{
diff --git a/posix/tst-execvp3.c b/posix/tst-execvp3.c
index 5ebc879..02a937c 100644
--- a/posix/tst-execvp3.c
+++ b/posix/tst-execvp3.c
@@ -12,6 +12,9 @@ static int do_test (void);
#include "../test-skeleton.c"
+#ifndef EXECVP
+# define EXECVP(file, argv) execvp (file, argv)
+#endif
static char *fname;
@@ -35,7 +38,7 @@ do_test (void)
}
char *argv[] = { fname, NULL };
- execvp (basename (fname), argv);
+ EXECVP (basename (fname), argv);
/* If we come here, the execvp call failed. */
return 1;
diff --git a/posix/tst-execvp4.c b/posix/tst-execvp4.c
index 531fab2..589a560 100644
--- a/posix/tst-execvp4.c
+++ b/posix/tst-execvp4.c
@@ -5,6 +5,10 @@
#include <unistd.h>
#include <sys/stat.h>
+#ifndef EXECVP
+# define EXECVP(file, argv) execvp (file, argv)
+#endif
+
static int
do_test (void)
{
@@ -27,7 +31,7 @@ do_test (void)
unsetenv ("PATH");
char *argv[] = { buf + 9, NULL };
- execvp (argv[0], argv);
+ EXECVP (argv[0], argv);
return 0;
}
diff --git a/posix/tst-execvpe1.c b/posix/tst-execvpe1.c
new file mode 100644
index 0000000..622eb79
--- /dev/null
+++ b/posix/tst-execvpe1.c
@@ -0,0 +1,20 @@
+/* Check ENOENT failure for execvpe.
+ 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/>. */
+
+#define EXECVP(file, argv) execvpe (file, argv, NULL)
+#include <posix/tst-execvp1.c>
diff --git a/posix/tst-execvpe2.c b/posix/tst-execvpe2.c
new file mode 100644
index 0000000..cdec09a
--- /dev/null
+++ b/posix/tst-execvpe2.c
@@ -0,0 +1,20 @@
+/* Check EACCES for execvpe.
+ 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/>. */
+
+#define EXECVP(file, argv) execvpe (file, argv, NULL)
+#include <posix/tst-execvp2.c>
diff --git a/posix/tst-execvpe3.c b/posix/tst-execvpe3.c
new file mode 100644
index 0000000..47cdc14
--- /dev/null
+++ b/posix/tst-execvpe3.c
@@ -0,0 +1,20 @@
+/* Check script execution without shebang for execvpe.
+ 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/>. */
+
+#define EXECVP(file, argv) execvpe (file, argv, NULL)
+#include <posix/tst-execvp3.c>
diff --git a/posix/tst-execvpe4.c b/posix/tst-execvpe4.c
new file mode 100644
index 0000000..e8883aa
--- /dev/null
+++ b/posix/tst-execvpe4.c
@@ -0,0 +1,20 @@
+/* Check unexistent binary for execvpe.
+ 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/>. */
+
+#define EXECVP(file, argv) execvpe (file, argv, NULL)
+#include <posix/tst-execvp4.c>
diff --git a/posix/tst-execvpe5.c b/posix/tst-execvpe5.c
new file mode 100644
index 0000000..ffd764a
--- /dev/null
+++ b/posix/tst-execvpe5.c
@@ -0,0 +1,157 @@
+/* General tests for execpve.
+ 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 <errno.h>
+#include <fcntl.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+#include <wait.h>
+
+
+/* Nonzero if the program gets called via `exec'. */
+static int restart;
+
+
+#define CMDLINE_OPTIONS \
+ { "restart", no_argument, &restart, 1 },
+
+/* Prototype for our test function. */
+extern void do_prepare (int argc, char *argv[]);
+extern int do_test (int argc, char *argv[]);
+
+#include "../test-skeleton.c"
+
+#define EXECVPE_KEY "EXECVPE_ENV"
+#define EXECVPE_VALUE "execvpe_test"
+
+
+static int
+handle_restart (void)
+{
+ /* First check if only one variable is passed on execvpe. */
+ int env_count = 0;
+ for (char **e = environ; *e != NULL; ++e)
+ if (++env_count == INT_MAX)
+ {
+ printf ("Environment variable number overflow");
+ exit (EXIT_FAILURE);
+ }
+ if (env_count != 1)
+ {
+ printf ("Wrong number of environment variables");
+ exit (EXIT_FAILURE);
+ }
+
+ /* Check if the combinarion os "EXECVPE_ENV=execvpe_test" */
+ const char *env = getenv (EXECVPE_KEY);
+ if (env == NULL)
+ {
+ printf ("Test environment variable not found");
+ exit (EXIT_FAILURE);
+ }
+
+ if (strncmp (env, EXECVPE_VALUE, sizeof (EXECVPE_VALUE)))
+ {
+ printf ("Test environment variable with wrong value");
+ exit (EXIT_FAILURE);
+ }
+
+ return 0;
+}
+
+
+int
+do_test (int argc, char *argv[])
+{
+ pid_t pid;
+ int status;
+
+ /* We must have
+ - one or four parameters left if called initially
+ + path for ld.so optional
+ + "--library-path" optional
+ + the library path optional
+ + the application name
+ */
+
+ if (restart)
+ {
+ if (argc != 1)
+ {
+ printf ("Wrong number of arguments (%d)\n", argc);
+ exit (EXIT_FAILURE);
+ }
+
+ return handle_restart ();
+ }
+
+ if (argc != 2 && argc != 5)
+ {
+ printf ("Wrong number of arguments (%d)\n", argc);
+ exit (EXIT_FAILURE);
+ }
+
+ /* We want to test the `execvpe' function. To do this we restart the
+ program with an additional parameter. */
+ pid = fork ();
+ if (pid == 0)
+ {
+ /* This is the child. Construct the command line. */
+
+ /* We cast here to char* because the test itself does not modify neither
+ the argument nor the environment list. */
+ char *envs[] = { (char*)(EXECVPE_KEY "=" EXECVPE_VALUE), NULL };
+ if (argc == 5)
+ {
+ char *args[] = { argv[1], argv[2], argv[3], argv[4],
+ (char *) "--direct", (char *) "--restart", NULL };
+ execvpe (args[0], args, envs);
+ }
+ else
+ {
+ char *args[] = { argv[1], argv[1],
+ (char *) "--direct", (char *) "--restart", NULL };
+ execvpe (args[0], args, envs);
+ }
+
+ puts ("Cannot exec");
+ exit (EXIT_FAILURE);
+ }
+ else if (pid == (pid_t) -1)
+ {
+ puts ("Cannot fork");
+ return 1;
+ }
+
+ /* Wait for the child. */
+ if (waitpid (pid, &status, 0) != pid)
+ {
+ puts ("Wrong child");
+ return 1;
+ }
+
+ if (WTERMSIG (status) != 0)
+ {
+ puts ("Child terminated incorrectly");
+ return 1;
+ }
+ status = WEXITSTATUS (status);
+
+ return status;
+}
diff --git a/posix/tst-execvpe6.c b/posix/tst-execvpe6.c
new file mode 100644
index 0000000..0c78dd0
--- /dev/null
+++ b/posix/tst-execvpe6.c
@@ -0,0 +1,150 @@
+/* Check execvpe script argument handling.
+ 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 <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+#include <sys/stat.h>
+#include <sys/param.h>
+
+static char *fname1;
+static char *fname2;
+static char *logname;
+
+static void do_prepare (void);
+#define PREPARE(argc, argv) do_prepare ()
+static int do_test (void);
+#define TEST_FUNCTION do_test ()
+
+#include "../test-skeleton.c"
+
+static void
+do_prepare (void)
+{
+ int logfd = create_temp_file ("logfile", &logname);
+ close (logfd);
+
+ int fd1 = create_temp_file ("testscript", &fname1);
+ dprintf (fd1, "echo foo $1 $2 $3 > %s\n", logname);
+ fchmod (fd1, 0700);
+ close (fd1);
+
+ int fd2 = create_temp_file ("testscript", &fname2);
+ dprintf (fd2, "echo foo > %s\n", logname);
+ fchmod (fd2, 0700);
+ close (fd2);
+}
+
+static int
+run_script (const char *fname, char *args[])
+{
+ /* We want to test the `execvpe' function. To do this we restart the
+ program with an additional parameter. */
+ int status;
+ pid_t pid = fork ();
+ if (pid == 0)
+ {
+ execvpe (fname, args, NULL);
+
+ puts ("Cannot exec");
+ exit (EXIT_FAILURE);
+ }
+ else if (pid == (pid_t) -1)
+ {
+ puts ("Cannot fork");
+ return 1;
+ }
+
+ /* Wait for the child. */
+ if (waitpid (pid, &status, 0) != pid)
+ {
+ puts ("Wrong child");
+ return 1;
+ }
+
+ if (WTERMSIG (status) != 0)
+ {
+ puts ("Child terminated incorrectly");
+ return 1;
+ }
+
+ return 0;
+}
+
+static int
+check_output (const char *expected)
+{
+ /* Check log output. */
+ FILE *arq = fopen (logname, "r");
+ if (arq == NULL)
+ {
+ puts ("Error opening output file");
+ return 1;
+ }
+
+ char line[128];
+ if (fgets (line, sizeof (line), arq) == NULL)
+ {
+ puts ("Error reading output file");
+ return 1;
+ }
+ fclose (arq);
+
+ if (strcmp (line, expected) != 0)
+ {
+ puts ("Output file different than expected");
+ return 1;
+ }
+
+ return 0;
+}
+
+static int
+do_test (void)
+{
+ if (setenv ("PATH", test_dir, 1) != 0)
+ {
+ puts ("setenv failed");
+ return 1;
+ }
+
+ /* First check resulting script run with some arguments results in correct
+ output file. */
+ char *args1[] = { fname1, (char*) "1", (char *) "2", (char *) "3", NULL };
+ if (run_script (fname1,args1))
+ return 1;
+ if (check_output ("foo 1 2 3\n"))
+ return 1;
+
+ /* Same as before but with an expected empty argument list. */
+ char *args2[] = { fname2 };
+ if (run_script (fname2, args2))
+ return 1;
+ if (check_output ("foo\n"))
+ return 1;
+
+ /* Same as before but with an empty argument list. */
+ char *args3[] = { NULL };
+ if (run_script (fname2, args3))
+ return 1;
+ if (check_output ("foo\n"))
+ return 1;
+
+ return 0;
+}
--
1.9.1
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 2/3] posix: execvpe cleanup
2016-02-28 3:31 ` Paul Eggert
@ 2016-02-29 17:45 ` Adhemerval Zanella
0 siblings, 0 replies; 24+ messages in thread
From: Adhemerval Zanella @ 2016-02-29 17:45 UTC (permalink / raw)
To: Paul Eggert, libc-alpha
On 27-02-2016 20:17, Paul Eggert wrote:
> Adhemerval Zanella wrote:
>> according to POSIX [1], "argv[0] should point to a filename string that
>> is associated with the process being started by one of the exec functions"
>
> Sure, but in this context the word "should" is talking about (as POSIX puts it) "a feature or behavior that is recommended programming practice for optimum portability." A conforming application is not required to follow the recommendation, and in order to conform to POSIX glibc must support applications that do not follow the recommendation but otherwise conform.
>
> My source for the above quote:
>
> http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap01.html#tag_01_05_06
Fair enough. I also noted uglibc also accepts the empty argument list.
I will change it.
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 2/3] posix: execvpe cleanup
2016-02-27 23:18 ` Adhemerval Zanella
@ 2016-02-28 3:31 ` Paul Eggert
2016-02-29 17:45 ` Adhemerval Zanella
0 siblings, 1 reply; 24+ messages in thread
From: Paul Eggert @ 2016-02-28 3:31 UTC (permalink / raw)
To: Adhemerval Zanella, libc-alpha
Adhemerval Zanella wrote:
> according to POSIX [1], "argv[0] should point to a filename string that
> is associated with the process being started by one of the exec functions"
Sure, but in this context the word "should" is talking about (as POSIX puts it)
"a feature or behavior that is recommended programming practice for optimum
portability." A conforming application is not required to follow the
recommendation, and in order to conform to POSIX glibc must support applications
that do not follow the recommendation but otherwise conform.
My source for the above quote:
http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap01.html#tag_01_05_06
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 2/3] posix: execvpe cleanup
2016-02-26 22:22 ` Adhemerval Zanella
@ 2016-02-27 23:18 ` Adhemerval Zanella
2016-02-28 3:31 ` Paul Eggert
0 siblings, 1 reply; 24+ messages in thread
From: Adhemerval Zanella @ 2016-02-27 23:18 UTC (permalink / raw)
To: Paul Eggert, libc-alpha
On 26-02-2016 17:49, Adhemerval Zanella wrote:
>
>
> On 26-02-2016 16:38, Paul Eggert wrote:
>> This one has similar problems with int vs ptrdiff_t. Also:
>>
>> On 02/26/2016 05:56 AM, Adhemerval Zanella wrote:
>>> for (const char *p = path; ; p = subp)
>>> {
>>> if (errno == ENOEXEC)
>>> maybe_script_execute (buffer, argv, envp);
>>
>> This has O(P*C) behavior, where P is the length of the path and C is the argument count. How about changing it to have O(P + C) behavior instead, by allocating the substitute argv in __execvpe, and reusing it each time through the loop? (Admittedly the current code also has this performance bug.)
>>
>
> I do not oppose, although I would like to focus on fixing the usability and
> conformance bugs first and set the performance goal as possible future
> improvement. I will add a comment about this possible optimization.
>
>>> /* Construct an argument list for the shell. */
>>> char *new_argv[argc];
>>
>> This should be "argc +1", not "argc". Shouldn't we have a test case to catch bugs like this?
>
> Indeed I will change that. I add a testcase.
>
>>
>>> new_argv[0] = (char *) _PATH_BSHELL;
>>> new_argv[1] = (char *) file;
>>> while (argc > 1)
>>> {
>>> new_argv[argc] = argv[argc - 1];
>>> --argc;
>>> }
>>
>> This mishandles the case where argc == 1, which is possible with an empty argument vector. The resulting argument vector is not null-terminated. (Admittedly the current code also has this bug.) I suppose we should have a test case for this.
>>
>
> I will fix that.
In fact I think original code is correct. Although execvpe is an GNU extension I
see it should follow the specification of already defined exec* POSIX functions.
And according to POSIX [1], "argv[0] should point to a filename string that
is associated with the process being started by one of the exec functions" (this
differ with man pages with uses the wording 'by convention' to define first
argument should point to process being stated).
So I see that:
char *args[] = { NULL };
execvpe (scriptname, args, NULL)
Which triggers the issue is invalid.
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 2/3] posix: execvpe cleanup
2016-02-26 19:40 ` Paul Eggert
@ 2016-02-26 22:22 ` Adhemerval Zanella
2016-02-27 23:18 ` Adhemerval Zanella
0 siblings, 1 reply; 24+ messages in thread
From: Adhemerval Zanella @ 2016-02-26 22:22 UTC (permalink / raw)
To: Paul Eggert, libc-alpha
On 26-02-2016 16:38, Paul Eggert wrote:
> This one has similar problems with int vs ptrdiff_t. Also:
>
> On 02/26/2016 05:56 AM, Adhemerval Zanella wrote:
>> for (const char *p = path; ; p = subp)
>> {
>> if (errno == ENOEXEC)
>> maybe_script_execute (buffer, argv, envp);
>
> This has O(P*C) behavior, where P is the length of the path and C is the argument count. How about changing it to have O(P + C) behavior instead, by allocating the substitute argv in __execvpe, and reusing it each time through the loop? (Admittedly the current code also has this performance bug.)
>
I do not oppose, although I would like to focus on fixing the usability and
conformance bugs first and set the performance goal as possible future
improvement. I will add a comment about this possible optimization.
>> /* Construct an argument list for the shell. */
>> char *new_argv[argc];
>
> This should be "argc +1", not "argc". Shouldn't we have a test case to catch bugs like this?
Indeed I will change that. I add a testcase.
>
>> new_argv[0] = (char *) _PATH_BSHELL;
>> new_argv[1] = (char *) file;
>> while (argc > 1)
>> {
>> new_argv[argc] = argv[argc - 1];
>> --argc;
>> }
>
> This mishandles the case where argc == 1, which is possible with an empty argument vector. The resulting argument vector is not null-terminated. (Admittedly the current code also has this bug.) I suppose we should have a test case for this.
>
I will fix that.
>> bool got_eacces = false;
>> for (const char *p = path; ; p = subp)
>> {
>> char buffer[path_len + file_len + 1];
>
> Declare 'buffer' just before the loop starts, not in the loop body. That will make the code run a bit faster, as the buffer will be allocated and deallocated only once. This is OK since (path_len + file_len + 1) does not change.
>
I will change that.
>> /* Set current path considered plus a '/'. */
>> memcpy (buffer, p, subp - p);
>> buffer[subp - p] = '/';
>> /* And the file to execute. */
>> memcpy (buffer + (subp - p) + !!(subp > p), file, file_len + 1);
>
> Shouldn't this be using mempcpy? That should simplify the code. I find the "!!" ugly and unnecessary; there's nothing wrong with treating a boolean as an int, and besides, !! on a boolean still gives you a boolean so what's the point? Something like this, perhaps:
>
> /* Use the current path entry, plus a '/' if nonempty, plus the file to execute. */
> char *pend = mempcpy (buffer, p, subp - p);
> *pend = '/';
> memcpy (pend + (p < subp), file, file_len + 1);
>
>
I do not have a preference regarding to "!!" and your suggestion seems better.
I will change that.
>> + /* Record the we got a 'Permission denied' error. If we end
>
> the -> that
Ack.
>
>> +# define EXECVP(__file, __argv) execvp (__file, __argv)
>
> No need for the underscores here.
OK, I will change that.
^ permalink raw reply [flat|nested] 24+ messages in thread
* Re: [PATCH 2/3] posix: execvpe cleanup
2016-02-26 13:57 ` [PATCH 2/3] posix: execvpe cleanup Adhemerval Zanella
@ 2016-02-26 19:40 ` Paul Eggert
2016-02-26 22:22 ` Adhemerval Zanella
0 siblings, 1 reply; 24+ messages in thread
From: Paul Eggert @ 2016-02-26 19:40 UTC (permalink / raw)
To: Adhemerval Zanella, libc-alpha
This one has similar problems with int vs ptrdiff_t. Also:
On 02/26/2016 05:56 AM, Adhemerval Zanella wrote:
> for (const char *p = path; ; p = subp)
> {
> if (errno == ENOEXEC)
> maybe_script_execute (buffer, argv, envp);
This has O(P*C) behavior, where P is the length of the path and C is the
argument count. How about changing it to have O(P + C) behavior instead,
by allocating the substitute argv in __execvpe, and reusing it each time
through the loop? (Admittedly the current code also has this performance
bug.)
> /* Construct an argument list for the shell. */
> char *new_argv[argc];
This should be "argc +1", not "argc". Shouldn't we have a test case to
catch bugs like this?
> new_argv[0] = (char *) _PATH_BSHELL;
> new_argv[1] = (char *) file;
> while (argc > 1)
> {
> new_argv[argc] = argv[argc - 1];
> --argc;
> }
This mishandles the case where argc == 1, which is possible with an
empty argument vector. The resulting argument vector is not
null-terminated. (Admittedly the current code also has this bug.) I
suppose we should have a test case for this.
> bool got_eacces = false;
> for (const char *p = path; ; p = subp)
> {
> char buffer[path_len + file_len + 1];
Declare 'buffer' just before the loop starts, not in the loop body. That
will make the code run a bit faster, as the buffer will be allocated and
deallocated only once. This is OK since (path_len + file_len + 1) does
not change.
> /* Set current path considered plus a '/'. */
> memcpy (buffer, p, subp - p);
> buffer[subp - p] = '/';
> /* And the file to execute. */
> memcpy (buffer + (subp - p) + !!(subp > p), file, file_len + 1);
Shouldn't this be using mempcpy? That should simplify the code. I find
the "!!" ugly and unnecessary; there's nothing wrong with treating a
boolean as an int, and besides, !! on a boolean still gives you a
boolean so what's the point? Something like this, perhaps:
/* Use the current path entry, plus a '/' if nonempty, plus the
file to execute. */
char *pend = mempcpy (buffer, p, subp - p);
*pend = '/';
memcpy (pend + (p < subp), file, file_len + 1);
> + /* Record the we got a 'Permission denied' error. If we end
the -> that
> +# define EXECVP(__file, __argv) execvp (__file, __argv)
No need for the underscores here.
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 2/3] posix: execvpe cleanup
2016-02-26 13:57 [PATCH 0/3] posix: Execute file function fixes Adhemerval Zanella
@ 2016-02-26 13:57 ` Adhemerval Zanella
2016-02-26 19:40 ` Paul Eggert
0 siblings, 1 reply; 24+ messages in thread
From: Adhemerval Zanella @ 2016-02-26 13:57 UTC (permalink / raw)
To: libc-alpha
This patch removes all the dynamic allocation on execvpe code and
instead use direct stack allocation. This is QoI approach to make
it possible use in scenarios where memory is shared with parent
(vfork or clone with CLONE_VM).
For default process spawn (script file without a shebang), stack
allocation is bounded by NAME_MAX plus PATH_MAX plus 1. Large
file arguments returns an error (ENAMETOOLONG). This differs than
current GLIBC pratice in general, but it used to limit stack
allocation for large inputs. Also, path in PATH environment variable
larger than PATH_MAX are ignored.
The shell direct execution exeception, where execve returns ENOEXEC,
might requires a large stack allocation due large input argument list.
Tested on i686, x86_64, powerpc64le, and aarch64.
* posix/execvpe.c (__execvpe): Remove dynamic allocation.
* posix/Makefile (tests): Add tst-execvpe{1,2,3,4,5,6}.
* posix/tst-execvp1.c (do_test): Use a macro to call execvp.
* posix/tst-execvp2.c (do_test): Likewise.
* posix/tst-execvp3.c (do_test): Likewise.
* posix/tst-execvp4.c (do_test): Likewise.
* posix/tst-execvpe1.c: New file.
* posix/tst-execvpe2.c: Likewise.
* posix/tst-execvpe3.c: Likewise.
* posix/tst-execvpe4.c: Likewise.
* posix/tst-execvpe5.c: Likewise.
* posix/tst-execvpe6.c: Likewise.
---
posix/Makefile | 3 +
posix/execvpe.c | 244 +++++++++++++++++++++------------------------------
posix/tst-execvp1.c | 6 +-
posix/tst-execvp2.c | 5 +-
posix/tst-execvp3.c | 5 +-
posix/tst-execvp4.c | 6 +-
posix/tst-execvpe1.c | 20 +++++
posix/tst-execvpe2.c | 20 +++++
posix/tst-execvpe3.c | 20 +++++
posix/tst-execvpe4.c | 20 +++++
posix/tst-execvpe5.c | 157 +++++++++++++++++++++++++++++++++
posix/tst-execvpe6.c | 82 +++++++++++++++++
13 files changed, 455 insertions(+), 146 deletions(-)
create mode 100644 posix/tst-execvpe1.c
create mode 100644 posix/tst-execvpe2.c
create mode 100644 posix/tst-execvpe3.c
create mode 100644 posix/tst-execvpe4.c
create mode 100644 posix/tst-execvpe5.c
create mode 100644 posix/tst-execvpe6.c
diff --git a/posix/Makefile b/posix/Makefile
index 55f4f31..a2aabcc 100644
--- a/posix/Makefile
+++ b/posix/Makefile
@@ -82,6 +82,8 @@ tests := tstgetopt testfnm runtests runptests \
tst-execv1 tst-execv2 tst-execl1 tst-execl2 \
tst-execve1 tst-execve2 tst-execle1 tst-execle2 \
tst-execvp3 tst-execvp4 tst-rfc3484 tst-rfc3484-2 \
+ tst-execvpe1 tst-execvpe2 tst-execvpe3 tst-execvpe4 \
+ tst-execvpe5 tst-execvpe6 \
tst-rfc3484-3 \
tst-getaddrinfo3 tst-fnmatch2 tst-cpucount tst-cpuset \
bug-getopt1 bug-getopt2 bug-getopt3 bug-getopt4 \
@@ -229,6 +231,7 @@ tstgetopt-ARGS = -a -b -cfoobar --required foobar --optional=bazbug \
tst-exec-ARGS = -- $(host-test-program-cmd)
tst-exec-static-ARGS = $(tst-exec-ARGS)
+tst-execvpe5-ARGS = -- $(host-test-program-cmd)
tst-spawn-ARGS = -- $(host-test-program-cmd)
tst-spawn-static-ARGS = $(tst-spawn-ARGS)
tst-dir-ARGS = `pwd` `cd $(common-objdir)/$(subdir); pwd` `cd $(common-objdir); pwd` $(objpfx)tst-dir
diff --git a/posix/execvpe.c b/posix/execvpe.c
index 61697a7..b945ef2 100644
--- a/posix/execvpe.c
+++ b/posix/execvpe.c
@@ -15,7 +15,6 @@
License along with the GNU C Library; if not, see
<http://www.gnu.org/licenses/>. */
-#include <alloca.h>
#include <unistd.h>
#include <stdarg.h>
#include <stdbool.h>
@@ -23,15 +22,34 @@
#include <string.h>
#include <errno.h>
#include <paths.h>
+#include <confstr.h>
+#include <sys/param.h>
+#ifndef PATH_MAX
+# ifdef MAXPATHLEN
+# define PATH_MAX MAXPATHLEN
+# else
+# define PATH_MAX 1024
+# endif
+#endif
/* The file is accessible but it is not an executable file. Invoke
the shell to interpret it as a script. */
static void
-internal_function
-scripts_argv (const char *file, char *const argv[], int argc, char **new_argv)
+maybe_script_execute (const char *file, char *const argv[], char *const envp[])
{
+ int argc = 0;
+ while (argv[argc++] != NULL)
+ {
+ if (argc == INT_MAX)
+ {
+ errno = E2BIG;
+ return;
+ }
+ }
+
/* Construct an argument list for the shell. */
+ char *new_argv[argc];
new_argv[0] = (char *) _PATH_BSHELL;
new_argv[1] = (char *) file;
while (argc > 1)
@@ -39,6 +57,9 @@ scripts_argv (const char *file, char *const argv[], int argc, char **new_argv)
new_argv[argc] = argv[argc - 1];
--argc;
}
+
+ /* Execute the shell. */
+ __execve (new_argv[0], new_argv, envp);
}
@@ -47,170 +68,109 @@ scripts_argv (const char *file, char *const argv[], int argc, char **new_argv)
int
__execvpe (const char *file, char *const argv[], char *const envp[])
{
+ /* We check the simple case first. */
if (*file == '\0')
{
- /* We check the simple case first. */
__set_errno (ENOENT);
return -1;
}
+ /* Don't search when it contains a slash. */
if (strchr (file, '/') != NULL)
{
- /* Don't search when it contains a slash. */
__execve (file, argv, envp);
if (errno == ENOEXEC)
- {
- /* Count the arguments. */
- int argc = 0;
- while (argv[argc++])
- ;
- size_t len = (argc + 1) * sizeof (char *);
- char **script_argv;
- void *ptr = NULL;
- if (__libc_use_alloca (len))
- script_argv = alloca (len);
- else
- script_argv = ptr = malloc (len);
-
- if (script_argv != NULL)
- {
- scripts_argv (file, argv, argc, script_argv);
- __execve (script_argv[0], script_argv, envp);
-
- free (ptr);
- }
- }
+ maybe_script_execute (file, argv, envp);
+
+ return -1;
}
- else
+
+ const char *path = getenv ("PATH");
+ if (!path)
+ path = CS_PATH;
+ /* Although GLIBC does not enforce NAME_MAX, we set it as the maximum
+ size to avoid unbounded stack allocation. Same applies for
+ PATH_MAX. */
+ size_t file_len = __strnlen (file, NAME_MAX + 1);
+ size_t path_len = __strnlen (path, PATH_MAX - 1) + 1;
+
+ if ((file_len > NAME_MAX)
+ || !__libc_alloca_cutoff (path_len + file_len + 1))
{
- size_t pathlen;
- size_t alloclen = 0;
- char *path = getenv ("PATH");
- if (path == NULL)
- {
- pathlen = confstr (_CS_PATH, (char *) NULL, 0);
- alloclen = pathlen + 1;
- }
- else
- pathlen = strlen (path);
+ errno = ENAMETOOLONG;
+ return -1;
+ }
- size_t len = strlen (file) + 1;
- alloclen += pathlen + len + 1;
+ const char *subp;
+ bool got_eacces = false;
+ for (const char *p = path; ; p = subp)
+ {
+ char buffer[path_len + file_len + 1];
- char *name;
- char *path_malloc = NULL;
- if (__libc_use_alloca (alloclen))
- name = alloca (alloclen);
- else
- {
- path_malloc = name = malloc (alloclen);
- if (name == NULL)
- return -1;
- }
+ subp = __strchrnul (p, ':');
- if (path == NULL)
+ /* PATH is larger than PATH_MAX and thus potentially larger than
+ the stack allocation. */
+ if (subp - p >= path_len)
{
- /* There is no `PATH' in the environment.
- The default search path is the current directory
- followed by the path `confstr' returns for `_CS_PATH'. */
- path = name + pathlen + len + 1;
- path[0] = ':';
- (void) confstr (_CS_PATH, path + 1, pathlen);
+ /* If there is only one path, bail out. */
+ if (*subp == '\0')
+ break;
+ /* Otherwise skip to next one. */
+ continue;
}
- /* Copy the file name at the top. */
- name = (char *) memcpy (name + pathlen + 1, file, len);
- /* And add the slash. */
- *--name = '/';
+ /* Set current path considered plus a '/'. */
+ memcpy (buffer, p, subp - p);
+ buffer[subp - p] = '/';
+ /* And the file to execute. */
+ memcpy (buffer + (subp - p) + !!(subp > p), file, file_len + 1);
+
+ __execve (buffer, argv, envp);
- char **script_argv = NULL;
- void *script_argv_malloc = NULL;
- bool got_eacces = false;
- char *p = path;
- do
+ if (errno == ENOEXEC)
+ maybe_script_execute (buffer, argv, envp);
+
+ switch (errno)
{
- char *startp;
-
- path = p;
- p = __strchrnul (path, ':');
-
- if (p == path)
- /* Two adjacent colons, or a colon at the beginning or the end
- of `PATH' means to search the current directory. */
- startp = name + 1;
- else
- startp = (char *) memcpy (name - (p - path), path, p - path);
-
- /* Try to execute this name. If it works, execve will not return. */
- __execve (startp, argv, envp);
-
- if (errno == ENOEXEC)
- {
- if (script_argv == NULL)
- {
- /* Count the arguments. */
- int argc = 0;
- while (argv[argc++])
- ;
- size_t arglen = (argc + 1) * sizeof (char *);
- if (__libc_use_alloca (alloclen + arglen))
- script_argv = alloca (arglen);
- else
- script_argv = script_argv_malloc = malloc (arglen);
- if (script_argv == NULL)
- {
- /* A possible EACCES error is not as important as
- the ENOMEM. */
- got_eacces = false;
- break;
- }
- scripts_argv (startp, argv, argc, script_argv);
- }
-
- __execve (script_argv[0], script_argv, envp);
- }
-
- switch (errno)
- {
- case EACCES:
- /* Record the we got a `Permission denied' error. If we end
- up finding no executable we can use, we want to diagnose
- that we did find one but were denied access. */
- got_eacces = true;
- case ENOENT:
- case ESTALE:
- case ENOTDIR:
- /* Those errors indicate the file is missing or not executable
- by us, in which case we want to just try the next path
- directory. */
- case ENODEV:
- case ETIMEDOUT:
- /* Some strange filesystems like AFS return even
- stranger error numbers. They cannot reasonably mean
- anything else so ignore those, too. */
- break;
-
- default:
- /* Some other error means we found an executable file, but
- something went wrong executing it; return the error to our
- caller. */
- return -1;
- }
+ case EACCES:
+ /* Record the we got a 'Permission denied' error. If we end
+ up finding no executable we can use, we want to diagnose
+ that we did find one but were denied access. */
+ got_eacces = true;
+ case ENOENT:
+ case ESTALE:
+ case ENOTDIR:
+ /* Those errors indicate the file is missing or not executable
+ by us, in which case we want to just try the next path
+ directory. */
+ case ENODEV:
+ case ETIMEDOUT:
+ /* Some strange filesystems like AFS return even
+ stranger error numbers. They cannot reasonably mean
+ anything else so ignore those, too. */
+ break;
+
+ default:
+ /* Some other error means we found an executable file, but
+ something went wrong executing it; return the error to our
+ caller. */
+ return -1;
}
- while (*p++ != '\0');
-
- /* We tried every element and none of them worked. */
- if (got_eacces)
- /* At least one failure was due to permissions, so report that
- error. */
- __set_errno (EACCES);
- free (script_argv_malloc);
- free (path_malloc);
+ if (*subp++ == '\0')
+ break;
}
- /* Return the error from the last attempt (probably ENOENT). */
+ /* We tried every element and none of them worked. */
+ if (got_eacces)
+ /* At least one failure was due to permissions, so report that
+ error. */
+ __set_errno (EACCES);
+
return -1;
+
}
+
weak_alias (__execvpe, execvpe)
diff --git a/posix/tst-execvp1.c b/posix/tst-execvp1.c
index ecc673d..fe98ce5 100644
--- a/posix/tst-execvp1.c
+++ b/posix/tst-execvp1.c
@@ -3,6 +3,10 @@
#include <stdlib.h>
#include <unistd.h>
+#ifndef EXECVP
+# define EXECVP(__file, __argv) execvp (__file, __argv)
+#endif
+
static int
do_test (void)
{
@@ -19,7 +23,7 @@ do_test (void)
char *argv[] = { (char *) "does-not-exist", NULL };
errno = 0;
- execvp (argv[0], argv);
+ EXECVP (argv[0], argv);
if (errno != ENOENT)
{
diff --git a/posix/tst-execvp2.c b/posix/tst-execvp2.c
index 7e0f5d8..9be8733 100644
--- a/posix/tst-execvp2.c
+++ b/posix/tst-execvp2.c
@@ -14,6 +14,9 @@ static int do_test (void);
#define TEST_FUNCTION do_test ()
#include "../test-skeleton.c"
+#ifndef EXECVP
+# define EXECVP(__file, __argv) execvp (__file, __argv)
+#endif
static char *copy;
@@ -70,7 +73,7 @@ do_test (void)
char *argv[] = { basename (copy), NULL };
errno = 0;
- execvp (argv[0], argv);
+ EXECVP (argv[0], argv);
if (errno != EACCES)
{
diff --git a/posix/tst-execvp3.c b/posix/tst-execvp3.c
index 5ebc879..43f7c34 100644
--- a/posix/tst-execvp3.c
+++ b/posix/tst-execvp3.c
@@ -12,6 +12,9 @@ static int do_test (void);
#include "../test-skeleton.c"
+#ifndef EXECVP
+# define EXECVP(__file, __argv) execvp (__file, __argv)
+#endif
static char *fname;
@@ -35,7 +38,7 @@ do_test (void)
}
char *argv[] = { fname, NULL };
- execvp (basename (fname), argv);
+ EXECVP (basename (fname), argv);
/* If we come here, the execvp call failed. */
return 1;
diff --git a/posix/tst-execvp4.c b/posix/tst-execvp4.c
index 531fab2..116624f 100644
--- a/posix/tst-execvp4.c
+++ b/posix/tst-execvp4.c
@@ -5,6 +5,10 @@
#include <unistd.h>
#include <sys/stat.h>
+#ifndef EXECVP
+# define EXECVP(__file, __argv) execvp (__file, __argv)
+#endif
+
static int
do_test (void)
{
@@ -27,7 +31,7 @@ do_test (void)
unsetenv ("PATH");
char *argv[] = { buf + 9, NULL };
- execvp (argv[0], argv);
+ EXECVP (argv[0], argv);
return 0;
}
diff --git a/posix/tst-execvpe1.c b/posix/tst-execvpe1.c
new file mode 100644
index 0000000..622eb79
--- /dev/null
+++ b/posix/tst-execvpe1.c
@@ -0,0 +1,20 @@
+/* Check ENOENT failure for execvpe.
+ 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/>. */
+
+#define EXECVP(file, argv) execvpe (file, argv, NULL)
+#include <posix/tst-execvp1.c>
diff --git a/posix/tst-execvpe2.c b/posix/tst-execvpe2.c
new file mode 100644
index 0000000..cdec09a
--- /dev/null
+++ b/posix/tst-execvpe2.c
@@ -0,0 +1,20 @@
+/* Check EACCES for execvpe.
+ 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/>. */
+
+#define EXECVP(file, argv) execvpe (file, argv, NULL)
+#include <posix/tst-execvp2.c>
diff --git a/posix/tst-execvpe3.c b/posix/tst-execvpe3.c
new file mode 100644
index 0000000..47cdc14
--- /dev/null
+++ b/posix/tst-execvpe3.c
@@ -0,0 +1,20 @@
+/* Check script execution without shebang for execvpe.
+ 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/>. */
+
+#define EXECVP(file, argv) execvpe (file, argv, NULL)
+#include <posix/tst-execvp3.c>
diff --git a/posix/tst-execvpe4.c b/posix/tst-execvpe4.c
new file mode 100644
index 0000000..e8883aa
--- /dev/null
+++ b/posix/tst-execvpe4.c
@@ -0,0 +1,20 @@
+/* Check unexistent binary for execvpe.
+ 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/>. */
+
+#define EXECVP(file, argv) execvpe (file, argv, NULL)
+#include <posix/tst-execvp4.c>
diff --git a/posix/tst-execvpe5.c b/posix/tst-execvpe5.c
new file mode 100644
index 0000000..81085ba
--- /dev/null
+++ b/posix/tst-execvpe5.c
@@ -0,0 +1,157 @@
+/* General tests for execpve.
+ 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 <errno.h>
+#include <fcntl.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+#include <wait.h>
+
+
+/* Nonzero if the program gets called via `exec'. */
+static int restart;
+
+
+#define CMDLINE_OPTIONS \
+ { "restart", no_argument, &restart, 1 },
+
+/* Prototype for our test function. */
+extern void do_prepare (int argc, char *argv[]);
+extern int do_test (int argc, char *argv[]);
+
+#include "../test-skeleton.c"
+
+#define EXECVPE_KEY "EXECVPE_ENV"
+#define EXECVPE_VALUE "execvpe_test"
+
+
+static int
+handle_restart (void)
+{
+ /* First check if only one variable is passed on execvpe. */
+ int env_count = 0;
+ for (char **e = environ; *e != NULL; ++e)
+ if (++env_count == INT_MAX)
+ {
+ printf ("Environment variable number overflow");
+ exit (EXIT_FAILURE);
+ }
+ if (env_count != 1)
+ {
+ printf ("Wrong number of environment variables");
+ exit (EXIT_FAILURE);
+ }
+
+ /* Check if the combinarion os "EXECVPE_ENV=execvpe_test" */
+ const char *env = getenv (EXECVPE_KEY);
+ if (env == NULL)
+ {
+ printf ("Test environment variable not found");
+ exit (EXIT_FAILURE);
+ }
+
+ if (strncmp (env, EXECVPE_VALUE, sizeof (EXECVPE_VALUE)))
+ {
+ printf ("Test environment variable with wrong value");
+ exit (EXIT_FAILURE);
+ }
+
+ return 0;
+}
+
+
+int
+do_test (int argc, char *argv[])
+{
+ pid_t pid;
+ int status;
+
+ /* We must have
+ - one or four parameters left if called initially
+ + path for ld.so optional
+ + "--library-path" optional
+ + the library path optional
+ + the application name
+ */
+
+ if (restart)
+ {
+ if (argc != 1)
+ {
+ printf ("Wrong number of arguments (%d)", argc);
+ exit (EXIT_FAILURE);
+ }
+
+ return handle_restart ();
+ }
+
+ if (argc != 2 && argc != 5)
+ {
+ printf ("Wrong number of arguments (%d)", argc);
+ exit (EXIT_FAILURE);
+ }
+
+ /* We want to test the `execvpe' function. To do this we restart the
+ program with an additional parameter. */
+ pid = fork ();
+ if (pid == 0)
+ {
+ /* This is the child. Construct the command line. */
+
+ /* We cast here to char* because the test itself does not modify neither
+ the argument nor the environment list. */
+ char *envs[] = { (char*)(EXECVPE_KEY "=" EXECVPE_VALUE), NULL };
+ if (argc == 5)
+ {
+ char *args[] = { argv[1], argv[2], argv[3], argv[4],
+ (char *) "--direct", (char *) "--restart", NULL };
+ execvpe (args[0], args, envs);
+ }
+ else
+ {
+ char *args[] = { argv[1], argv[1],
+ (char *) "--direct", (char *) "--restart", NULL };
+ execvpe (args[0], args, envs);
+ }
+
+ printf ("Cannot exec");
+ exit (EXIT_FAILURE);
+ }
+ else if (pid == (pid_t) -1)
+ {
+ printf ("Cannot fork");
+ exit (EXIT_FAILURE);
+ }
+
+ /* Wait for the child. */
+ if (waitpid (pid, &status, 0) != pid)
+ {
+ printf ("Wrong child");
+ exit (EXIT_FAILURE);
+ }
+
+ if (WTERMSIG (status) != 0)
+ {
+ printf ("Child terminated incorrectly");
+ exit (EXIT_FAILURE);
+ }
+ status = WEXITSTATUS (status);
+
+ return status;
+}
diff --git a/posix/tst-execvpe6.c b/posix/tst-execvpe6.c
new file mode 100644
index 0000000..4551e2c
--- /dev/null
+++ b/posix/tst-execvpe6.c
@@ -0,0 +1,82 @@
+/* Check execvpe script argument handling.
+ 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 <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+#include <sys/stat.h>
+#include <sys/param.h>
+
+static char *fname;
+
+static void do_prepare (void);
+#define PREPARE(argc, argv) do_prepare ()
+static int do_test (void);
+#define TEST_FUNCTION do_test ()
+
+#include "../test-skeleton.c"
+
+static void
+do_prepare (void)
+{
+ int fd = create_temp_file ("testscript", &fname);
+ dprintf (fd, "echo foo\n");
+ fchmod (fd, 0700);
+ close (fd);
+}
+
+static int
+do_test (void)
+{
+ if (setenv ("PATH", test_dir, 1) != 0)
+ {
+ puts ("setenv failed");
+ return 1;
+ }
+
+ /* To limit stack allocation for argument construction in case of
+ script without shebang execvpe limits total number of argument
+ to NCARGS. */
+ const size_t max_args = NCARGS + 1;
+ char **args = malloc ((NCARGS + 1) * sizeof (char*));
+ if (args == NULL)
+ {
+ puts ("malloc failed");
+ return 1;
+ }
+
+ args[0] = fname;
+ for (int i = 1; i < max_args; ++i)
+ args[i] = strdup ("a");
+ args[max_args] = NULL;
+
+ execvpe (basename (fname), args, NULL);
+
+ for (int i = 1; i < max_args; ++i)
+ free (args[i]);
+ free (args);
+
+ if (errno != E2BIG)
+ {
+ printf ("errno = %d (%m), expected E2BIG\n", errno);
+ return 1;
+ }
+
+ return 0;
+}
--
1.9.1
^ permalink raw reply [flat|nested] 24+ messages in thread
* [PATCH 2/3] posix: execvpe cleanup
2016-02-22 13:03 [PATCH v3 " Adhemerval Zanella
@ 2016-02-22 13:03 ` Adhemerval Zanella
0 siblings, 0 replies; 24+ messages in thread
From: Adhemerval Zanella @ 2016-02-22 13:03 UTC (permalink / raw)
To: libc-alpha
This patch removes all the dynamic allocation on execvpe code and
instead use direct stack allocation. This is QoI approach to make
it possible use in scenarios where memory is shared with parent
(vfork or clone with CLONE_VM).
For default process spawn (script file without a shebang), stack
allocation is bounded by NAME_MAX plus PATH_MAX plus 1. Large
file arguments returns an error (ENAMETOOLONG). This differs than
current GLIBC pratice in general, but it used to limit stack
allocation for large inputs. Also, path in PATH environment variable
larger than PATH_MAX are ignored.
The shell direct execution exeception, where execve returns ENOEXEC,
might requires a large stack allocation due large input argument list.
Tested on i686, x86_64, powerpc64le, and aarch64.
* posix/execvpe.c (__execvpe): Remove dynamic allocation.
* posix/Makefile (tests): Add tst-execvpe{1,2,3,4,5,6}.
* posix/tst-execvp1.c (do_test): Use a macro to call execvp.
* posix/tst-execvp2.c (do_test): Likewise.
* posix/tst-execvp3.c (do_test): Likewise.
* posix/tst-execvp4.c (do_test): Likewise.
* posix/tst-execvpe1.c: New file.
* posix/tst-execvpe2.c: Likewise.
* posix/tst-execvpe3.c: Likewise.
* posix/tst-execvpe4.c: Likewise.
* posix/tst-execvpe5.c: Likewise.
* posix/tst-execvpe6.c: Likewise.
---
posix/Makefile | 3 +
posix/execvpe.c | 238 +++++++++++++++++++++------------------------------
posix/tst-execvp1.c | 6 +-
posix/tst-execvp2.c | 5 +-
posix/tst-execvp3.c | 5 +-
posix/tst-execvp4.c | 6 +-
posix/tst-execvpe1.c | 20 +++++
posix/tst-execvpe2.c | 20 +++++
posix/tst-execvpe3.c | 20 +++++
posix/tst-execvpe4.c | 20 +++++
posix/tst-execvpe5.c | 157 +++++++++++++++++++++++++++++++++
posix/tst-execvpe6.c | 82 ++++++++++++++++++
13 files changed, 449 insertions(+), 146 deletions(-)
create mode 100644 posix/tst-execvpe1.c
create mode 100644 posix/tst-execvpe2.c
create mode 100644 posix/tst-execvpe3.c
create mode 100644 posix/tst-execvpe4.c
create mode 100644 posix/tst-execvpe5.c
create mode 100644 posix/tst-execvpe6.c
diff --git a/posix/Makefile b/posix/Makefile
index 55f4f31..a2aabcc 100644
--- a/posix/Makefile
+++ b/posix/Makefile
@@ -82,6 +82,8 @@ tests := tstgetopt testfnm runtests runptests \
tst-execv1 tst-execv2 tst-execl1 tst-execl2 \
tst-execve1 tst-execve2 tst-execle1 tst-execle2 \
tst-execvp3 tst-execvp4 tst-rfc3484 tst-rfc3484-2 \
+ tst-execvpe1 tst-execvpe2 tst-execvpe3 tst-execvpe4 \
+ tst-execvpe5 tst-execvpe6 \
tst-rfc3484-3 \
tst-getaddrinfo3 tst-fnmatch2 tst-cpucount tst-cpuset \
bug-getopt1 bug-getopt2 bug-getopt3 bug-getopt4 \
@@ -229,6 +231,7 @@ tstgetopt-ARGS = -a -b -cfoobar --required foobar --optional=bazbug \
tst-exec-ARGS = -- $(host-test-program-cmd)
tst-exec-static-ARGS = $(tst-exec-ARGS)
+tst-execvpe5-ARGS = -- $(host-test-program-cmd)
tst-spawn-ARGS = -- $(host-test-program-cmd)
tst-spawn-static-ARGS = $(tst-spawn-ARGS)
tst-dir-ARGS = `pwd` `cd $(common-objdir)/$(subdir); pwd` `cd $(common-objdir); pwd` $(objpfx)tst-dir
diff --git a/posix/execvpe.c b/posix/execvpe.c
index 61697a7..b8bb563 100644
--- a/posix/execvpe.c
+++ b/posix/execvpe.c
@@ -15,7 +15,6 @@
License along with the GNU C Library; if not, see
<http://www.gnu.org/licenses/>. */
-#include <alloca.h>
#include <unistd.h>
#include <stdarg.h>
#include <stdbool.h>
@@ -23,15 +22,28 @@
#include <string.h>
#include <errno.h>
#include <paths.h>
+#include <confstr.h>
+#include <sys/param.h>
+#ifndef PATH_MAX
+# ifdef MAXPATHLEN
+# define PATH_MAX MAXPATHLEN
+# else
+# define PATH_MAX 1024
+# endif
+#endif
/* The file is accessible but it is not an executable file. Invoke
the shell to interpret it as a script. */
static void
-internal_function
-scripts_argv (const char *file, char *const argv[], int argc, char **new_argv)
+maybe_script_execute (const char *file, char *const argv[], char *const envp[])
{
+ int argc = 0;
+ while (argv[argc++] != NULL)
+ continue;
+
/* Construct an argument list for the shell. */
+ char *new_argv[argc];
new_argv[0] = (char *) _PATH_BSHELL;
new_argv[1] = (char *) file;
while (argc > 1)
@@ -39,6 +51,9 @@ scripts_argv (const char *file, char *const argv[], int argc, char **new_argv)
new_argv[argc] = argv[argc - 1];
--argc;
}
+
+ /* Execute the shell. */
+ __execve (new_argv[0], new_argv, envp);
}
@@ -47,170 +62,109 @@ scripts_argv (const char *file, char *const argv[], int argc, char **new_argv)
int
__execvpe (const char *file, char *const argv[], char *const envp[])
{
+ /* We check the simple case first. */
if (*file == '\0')
{
- /* We check the simple case first. */
__set_errno (ENOENT);
return -1;
}
+ /* Don't search when it contains a slash. */
if (strchr (file, '/') != NULL)
{
- /* Don't search when it contains a slash. */
__execve (file, argv, envp);
if (errno == ENOEXEC)
- {
- /* Count the arguments. */
- int argc = 0;
- while (argv[argc++])
- ;
- size_t len = (argc + 1) * sizeof (char *);
- char **script_argv;
- void *ptr = NULL;
- if (__libc_use_alloca (len))
- script_argv = alloca (len);
- else
- script_argv = ptr = malloc (len);
-
- if (script_argv != NULL)
- {
- scripts_argv (file, argv, argc, script_argv);
- __execve (script_argv[0], script_argv, envp);
-
- free (ptr);
- }
- }
+ maybe_script_execute (file, argv, envp);
+
+ return -1;
}
- else
+
+ const char *path = getenv ("PATH");
+ if (!path)
+ path = CS_PATH;
+ /* Although GLIBC does not enforce NAME_MAX, we set it as the maximum
+ size to avoid unbounded stack allocation. Same applies for
+ PATH_MAX. */
+ size_t file_len = __strnlen (file, NAME_MAX + 1);
+ size_t path_len = __strnlen (path, PATH_MAX - 1) + 1;
+
+ if ((file_len > NAME_MAX)
+ || !__libc_alloca_cutoff (path_len + file_len + 1))
{
- size_t pathlen;
- size_t alloclen = 0;
- char *path = getenv ("PATH");
- if (path == NULL)
- {
- pathlen = confstr (_CS_PATH, (char *) NULL, 0);
- alloclen = pathlen + 1;
- }
- else
- pathlen = strlen (path);
+ errno = ENAMETOOLONG;
+ return -1;
+ }
- size_t len = strlen (file) + 1;
- alloclen += pathlen + len + 1;
+ const char *subp;
+ bool got_eacces = false;
+ for (const char *p = path; ; p = subp)
+ {
+ char buffer[path_len + file_len + 1];
- char *name;
- char *path_malloc = NULL;
- if (__libc_use_alloca (alloclen))
- name = alloca (alloclen);
- else
- {
- path_malloc = name = malloc (alloclen);
- if (name == NULL)
- return -1;
- }
+ subp = __strchrnul (p, ':');
- if (path == NULL)
+ /* PATH is larger than PATH_MAX and thus potentially larger than
+ the stack allocation. */
+ if (subp - p >= path_len)
{
- /* There is no `PATH' in the environment.
- The default search path is the current directory
- followed by the path `confstr' returns for `_CS_PATH'. */
- path = name + pathlen + len + 1;
- path[0] = ':';
- (void) confstr (_CS_PATH, path + 1, pathlen);
+ /* If there is only one path, bail out. */
+ if (*subp == '\0')
+ break;
+ /* Otherwise skip to next one. */
+ continue;
}
- /* Copy the file name at the top. */
- name = (char *) memcpy (name + pathlen + 1, file, len);
- /* And add the slash. */
- *--name = '/';
+ /* Set current path considered plus a '/'. */
+ memcpy (buffer, p, subp - p);
+ buffer[subp - p] = '/';
+ /* And the file to execute. */
+ memcpy (buffer + (subp - p) + !!(subp > p), file, file_len + 1);
+
+ __execve (buffer, argv, envp);
+
+ if (errno == ENOEXEC)
+ maybe_script_execute (buffer, argv, envp);
- char **script_argv = NULL;
- void *script_argv_malloc = NULL;
- bool got_eacces = false;
- char *p = path;
- do
+ switch (errno)
{
- char *startp;
-
- path = p;
- p = __strchrnul (path, ':');
-
- if (p == path)
- /* Two adjacent colons, or a colon at the beginning or the end
- of `PATH' means to search the current directory. */
- startp = name + 1;
- else
- startp = (char *) memcpy (name - (p - path), path, p - path);
-
- /* Try to execute this name. If it works, execve will not return. */
- __execve (startp, argv, envp);
-
- if (errno == ENOEXEC)
- {
- if (script_argv == NULL)
- {
- /* Count the arguments. */
- int argc = 0;
- while (argv[argc++])
- ;
- size_t arglen = (argc + 1) * sizeof (char *);
- if (__libc_use_alloca (alloclen + arglen))
- script_argv = alloca (arglen);
- else
- script_argv = script_argv_malloc = malloc (arglen);
- if (script_argv == NULL)
- {
- /* A possible EACCES error is not as important as
- the ENOMEM. */
- got_eacces = false;
- break;
- }
- scripts_argv (startp, argv, argc, script_argv);
- }
-
- __execve (script_argv[0], script_argv, envp);
- }
-
- switch (errno)
- {
- case EACCES:
- /* Record the we got a `Permission denied' error. If we end
- up finding no executable we can use, we want to diagnose
- that we did find one but were denied access. */
- got_eacces = true;
- case ENOENT:
- case ESTALE:
- case ENOTDIR:
- /* Those errors indicate the file is missing or not executable
- by us, in which case we want to just try the next path
- directory. */
- case ENODEV:
- case ETIMEDOUT:
- /* Some strange filesystems like AFS return even
- stranger error numbers. They cannot reasonably mean
- anything else so ignore those, too. */
- break;
-
- default:
- /* Some other error means we found an executable file, but
- something went wrong executing it; return the error to our
- caller. */
- return -1;
- }
+ case EACCES:
+ /* Record the we got a 'Permission denied' error. If we end
+ up finding no executable we can use, we want to diagnose
+ that we did find one but were denied access. */
+ got_eacces = true;
+ case ENOENT:
+ case ESTALE:
+ case ENOTDIR:
+ /* Those errors indicate the file is missing or not executable
+ by us, in which case we want to just try the next path
+ directory. */
+ case ENODEV:
+ case ETIMEDOUT:
+ /* Some strange filesystems like AFS return even
+ stranger error numbers. They cannot reasonably mean
+ anything else so ignore those, too. */
+ break;
+
+ default:
+ /* Some other error means we found an executable file, but
+ something went wrong executing it; return the error to our
+ caller. */
+ return -1;
}
- while (*p++ != '\0');
-
- /* We tried every element and none of them worked. */
- if (got_eacces)
- /* At least one failure was due to permissions, so report that
- error. */
- __set_errno (EACCES);
- free (script_argv_malloc);
- free (path_malloc);
+ if (*subp++ == '\0')
+ break;
}
- /* Return the error from the last attempt (probably ENOENT). */
+ /* We tried every element and none of them worked. */
+ if (got_eacces)
+ /* At least one failure was due to permissions, so report that
+ error. */
+ __set_errno (EACCES);
+
return -1;
+
}
+
weak_alias (__execvpe, execvpe)
diff --git a/posix/tst-execvp1.c b/posix/tst-execvp1.c
index ecc673d..fe98ce5 100644
--- a/posix/tst-execvp1.c
+++ b/posix/tst-execvp1.c
@@ -3,6 +3,10 @@
#include <stdlib.h>
#include <unistd.h>
+#ifndef EXECVP
+# define EXECVP(__file, __argv) execvp (__file, __argv)
+#endif
+
static int
do_test (void)
{
@@ -19,7 +23,7 @@ do_test (void)
char *argv[] = { (char *) "does-not-exist", NULL };
errno = 0;
- execvp (argv[0], argv);
+ EXECVP (argv[0], argv);
if (errno != ENOENT)
{
diff --git a/posix/tst-execvp2.c b/posix/tst-execvp2.c
index 7e0f5d8..9be8733 100644
--- a/posix/tst-execvp2.c
+++ b/posix/tst-execvp2.c
@@ -14,6 +14,9 @@ static int do_test (void);
#define TEST_FUNCTION do_test ()
#include "../test-skeleton.c"
+#ifndef EXECVP
+# define EXECVP(__file, __argv) execvp (__file, __argv)
+#endif
static char *copy;
@@ -70,7 +73,7 @@ do_test (void)
char *argv[] = { basename (copy), NULL };
errno = 0;
- execvp (argv[0], argv);
+ EXECVP (argv[0], argv);
if (errno != EACCES)
{
diff --git a/posix/tst-execvp3.c b/posix/tst-execvp3.c
index 5ebc879..43f7c34 100644
--- a/posix/tst-execvp3.c
+++ b/posix/tst-execvp3.c
@@ -12,6 +12,9 @@ static int do_test (void);
#include "../test-skeleton.c"
+#ifndef EXECVP
+# define EXECVP(__file, __argv) execvp (__file, __argv)
+#endif
static char *fname;
@@ -35,7 +38,7 @@ do_test (void)
}
char *argv[] = { fname, NULL };
- execvp (basename (fname), argv);
+ EXECVP (basename (fname), argv);
/* If we come here, the execvp call failed. */
return 1;
diff --git a/posix/tst-execvp4.c b/posix/tst-execvp4.c
index 531fab2..116624f 100644
--- a/posix/tst-execvp4.c
+++ b/posix/tst-execvp4.c
@@ -5,6 +5,10 @@
#include <unistd.h>
#include <sys/stat.h>
+#ifndef EXECVP
+# define EXECVP(__file, __argv) execvp (__file, __argv)
+#endif
+
static int
do_test (void)
{
@@ -27,7 +31,7 @@ do_test (void)
unsetenv ("PATH");
char *argv[] = { buf + 9, NULL };
- execvp (argv[0], argv);
+ EXECVP (argv[0], argv);
return 0;
}
diff --git a/posix/tst-execvpe1.c b/posix/tst-execvpe1.c
new file mode 100644
index 0000000..622eb79
--- /dev/null
+++ b/posix/tst-execvpe1.c
@@ -0,0 +1,20 @@
+/* Check ENOENT failure for execvpe.
+ 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/>. */
+
+#define EXECVP(file, argv) execvpe (file, argv, NULL)
+#include <posix/tst-execvp1.c>
diff --git a/posix/tst-execvpe2.c b/posix/tst-execvpe2.c
new file mode 100644
index 0000000..cdec09a
--- /dev/null
+++ b/posix/tst-execvpe2.c
@@ -0,0 +1,20 @@
+/* Check EACCES for execvpe.
+ 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/>. */
+
+#define EXECVP(file, argv) execvpe (file, argv, NULL)
+#include <posix/tst-execvp2.c>
diff --git a/posix/tst-execvpe3.c b/posix/tst-execvpe3.c
new file mode 100644
index 0000000..47cdc14
--- /dev/null
+++ b/posix/tst-execvpe3.c
@@ -0,0 +1,20 @@
+/* Check script execution without shebang for execvpe.
+ 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/>. */
+
+#define EXECVP(file, argv) execvpe (file, argv, NULL)
+#include <posix/tst-execvp3.c>
diff --git a/posix/tst-execvpe4.c b/posix/tst-execvpe4.c
new file mode 100644
index 0000000..e8883aa
--- /dev/null
+++ b/posix/tst-execvpe4.c
@@ -0,0 +1,20 @@
+/* Check unexistent binary for execvpe.
+ 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/>. */
+
+#define EXECVP(file, argv) execvpe (file, argv, NULL)
+#include <posix/tst-execvp4.c>
diff --git a/posix/tst-execvpe5.c b/posix/tst-execvpe5.c
new file mode 100644
index 0000000..81085ba
--- /dev/null
+++ b/posix/tst-execvpe5.c
@@ -0,0 +1,157 @@
+/* General tests for execpve.
+ 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 <errno.h>
+#include <fcntl.h>
+#include <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+#include <wait.h>
+
+
+/* Nonzero if the program gets called via `exec'. */
+static int restart;
+
+
+#define CMDLINE_OPTIONS \
+ { "restart", no_argument, &restart, 1 },
+
+/* Prototype for our test function. */
+extern void do_prepare (int argc, char *argv[]);
+extern int do_test (int argc, char *argv[]);
+
+#include "../test-skeleton.c"
+
+#define EXECVPE_KEY "EXECVPE_ENV"
+#define EXECVPE_VALUE "execvpe_test"
+
+
+static int
+handle_restart (void)
+{
+ /* First check if only one variable is passed on execvpe. */
+ int env_count = 0;
+ for (char **e = environ; *e != NULL; ++e)
+ if (++env_count == INT_MAX)
+ {
+ printf ("Environment variable number overflow");
+ exit (EXIT_FAILURE);
+ }
+ if (env_count != 1)
+ {
+ printf ("Wrong number of environment variables");
+ exit (EXIT_FAILURE);
+ }
+
+ /* Check if the combinarion os "EXECVPE_ENV=execvpe_test" */
+ const char *env = getenv (EXECVPE_KEY);
+ if (env == NULL)
+ {
+ printf ("Test environment variable not found");
+ exit (EXIT_FAILURE);
+ }
+
+ if (strncmp (env, EXECVPE_VALUE, sizeof (EXECVPE_VALUE)))
+ {
+ printf ("Test environment variable with wrong value");
+ exit (EXIT_FAILURE);
+ }
+
+ return 0;
+}
+
+
+int
+do_test (int argc, char *argv[])
+{
+ pid_t pid;
+ int status;
+
+ /* We must have
+ - one or four parameters left if called initially
+ + path for ld.so optional
+ + "--library-path" optional
+ + the library path optional
+ + the application name
+ */
+
+ if (restart)
+ {
+ if (argc != 1)
+ {
+ printf ("Wrong number of arguments (%d)", argc);
+ exit (EXIT_FAILURE);
+ }
+
+ return handle_restart ();
+ }
+
+ if (argc != 2 && argc != 5)
+ {
+ printf ("Wrong number of arguments (%d)", argc);
+ exit (EXIT_FAILURE);
+ }
+
+ /* We want to test the `execvpe' function. To do this we restart the
+ program with an additional parameter. */
+ pid = fork ();
+ if (pid == 0)
+ {
+ /* This is the child. Construct the command line. */
+
+ /* We cast here to char* because the test itself does not modify neither
+ the argument nor the environment list. */
+ char *envs[] = { (char*)(EXECVPE_KEY "=" EXECVPE_VALUE), NULL };
+ if (argc == 5)
+ {
+ char *args[] = { argv[1], argv[2], argv[3], argv[4],
+ (char *) "--direct", (char *) "--restart", NULL };
+ execvpe (args[0], args, envs);
+ }
+ else
+ {
+ char *args[] = { argv[1], argv[1],
+ (char *) "--direct", (char *) "--restart", NULL };
+ execvpe (args[0], args, envs);
+ }
+
+ printf ("Cannot exec");
+ exit (EXIT_FAILURE);
+ }
+ else if (pid == (pid_t) -1)
+ {
+ printf ("Cannot fork");
+ exit (EXIT_FAILURE);
+ }
+
+ /* Wait for the child. */
+ if (waitpid (pid, &status, 0) != pid)
+ {
+ printf ("Wrong child");
+ exit (EXIT_FAILURE);
+ }
+
+ if (WTERMSIG (status) != 0)
+ {
+ printf ("Child terminated incorrectly");
+ exit (EXIT_FAILURE);
+ }
+ status = WEXITSTATUS (status);
+
+ return status;
+}
diff --git a/posix/tst-execvpe6.c b/posix/tst-execvpe6.c
new file mode 100644
index 0000000..4551e2c
--- /dev/null
+++ b/posix/tst-execvpe6.c
@@ -0,0 +1,82 @@
+/* Check execvpe script argument handling.
+ 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 <stdlib.h>
+#include <string.h>
+#include <unistd.h>
+#include <sys/stat.h>
+#include <sys/param.h>
+
+static char *fname;
+
+static void do_prepare (void);
+#define PREPARE(argc, argv) do_prepare ()
+static int do_test (void);
+#define TEST_FUNCTION do_test ()
+
+#include "../test-skeleton.c"
+
+static void
+do_prepare (void)
+{
+ int fd = create_temp_file ("testscript", &fname);
+ dprintf (fd, "echo foo\n");
+ fchmod (fd, 0700);
+ close (fd);
+}
+
+static int
+do_test (void)
+{
+ if (setenv ("PATH", test_dir, 1) != 0)
+ {
+ puts ("setenv failed");
+ return 1;
+ }
+
+ /* To limit stack allocation for argument construction in case of
+ script without shebang execvpe limits total number of argument
+ to NCARGS. */
+ const size_t max_args = NCARGS + 1;
+ char **args = malloc ((NCARGS + 1) * sizeof (char*));
+ if (args == NULL)
+ {
+ puts ("malloc failed");
+ return 1;
+ }
+
+ args[0] = fname;
+ for (int i = 1; i < max_args; ++i)
+ args[i] = strdup ("a");
+ args[max_args] = NULL;
+
+ execvpe (basename (fname), args, NULL);
+
+ for (int i = 1; i < max_args; ++i)
+ free (args[i]);
+ free (args);
+
+ if (errno != E2BIG)
+ {
+ printf ("errno = %d (%m), expected E2BIG\n", errno);
+ return 1;
+ }
+
+ return 0;
+}
--
1.9.1
^ permalink raw reply [flat|nested] 24+ messages in thread
end of thread, other threads:[~2016-02-29 19:10 UTC | newest]
Thread overview: 24+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-02-19 18:05 [PATCH v2 0/3] posix: Execute file function fixes Adhemerval Zanella
2016-02-19 18:05 ` [PATCH 2/3] posix: execvpe cleanup Adhemerval Zanella
2016-02-20 8:32 ` Mike Frysinger
2016-02-19 18:05 ` [PATCH 1/3] posix: Remove dynamic memory allocation from execl{e,p} Adhemerval Zanella
2016-02-20 8:26 ` Mike Frysinger
2016-02-19 18:06 ` [PATCH 3/3] posix: New Linux posix_spawn{p} implementation Adhemerval Zanella
2016-02-19 18:33 ` [PATCH v2 0/3] posix: Execute file function fixes Paul Eggert
2016-02-19 19:14 ` Adhemerval Zanella
2016-02-19 19:58 ` Paul Eggert
2016-02-19 20:19 ` Adhemerval Zanella
2016-02-19 23:13 ` Paul Eggert
2016-02-20 10:37 ` Adhemerval Zanella
2016-02-20 18:25 ` Paul Eggert
2016-02-19 23:12 ` Joseph Myers
2016-02-19 23:26 ` Paul Eggert
2016-02-22 13:03 [PATCH v3 " Adhemerval Zanella
2016-02-22 13:03 ` [PATCH 2/3] posix: execvpe cleanup Adhemerval Zanella
2016-02-26 13:57 [PATCH 0/3] posix: Execute file function fixes Adhemerval Zanella
2016-02-26 13:57 ` [PATCH 2/3] posix: execvpe cleanup Adhemerval Zanella
2016-02-26 19:40 ` Paul Eggert
2016-02-26 22:22 ` Adhemerval Zanella
2016-02-27 23:18 ` Adhemerval Zanella
2016-02-28 3:31 ` Paul Eggert
2016-02-29 17:45 ` Adhemerval Zanella
2016-02-29 18:33 [PATCH v5 0/3] posix: Execute file function fixes Adhemerval Zanella
2016-02-29 18:34 ` [PATCH 2/3] posix: execvpe cleanup Adhemerval Zanella
2016-02-29 19:34 ` Paul Eggert
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).