posix: Use posix_spawn for wordexp

This patch replaces the fork+exec by posix_spawn on wordexp, which
allows a better scability on Linux and simplifies the thread
cancellation handling.

The only change which can not be implemented with posix_spawn the
/dev/null check to certify it is indeed the expected device.  I am
not sure how effetive this check is since /dev/null tampering means
something very wrong with the system and this is the least of the
issues.  My view is the tests is really out of the place and the
hardening provided is minimum.

If the idea is still to provide such check, I think a possibilty
would be to open /dev/null, check it, add a dup2 file action, and
close the file descriptor.

Checked on powerpc64le-linux-gnu and x86_64-linux-gnu.

	* include/spawn.h (__posix_spawn_file_actions_addopen): New
	prototype.
	* posix/spawn_faction_addopen.c (posix_spawn_file_actions_addopen):
	Add internal alias.
	* posix/wordexp.c (create_environment, free_environment): New
	functions.
	(exec_comm_child, exec_comm): Use posix_spawn instead of fork+exec.
	* posix/wordexp-test.c: Use libsupport.

1 files changed, 65 insertions, 86 deletions

diff --git a/posix/wordexp.c b/posix/wordexp.c
index 6a6e3a8..2aa6085 100644
--- a/posix/wordexp.c
+++ b/posix/wordexp.c
@@ -25,33 +25,18 @@
 #include <libintl.h>
 #include <paths.h>
 #include <pwd.h>
-#include <signal.h>
 #include <stdbool.h>
 #include <stdio.h>
-#include <stdlib.h>
 #include <string.h>
 #include <sys/param.h>
-#include <sys/stat.h>
-#include <sys/time.h>
-#include <sys/types.h>
-#include <sys/types.h>
 #include <sys/wait.h>
 #include <unistd.h>
-#include <wchar.h>
 #include <wordexp.h>
-#include <kernel-features.h>
+#include <spawn.h>
 #include <scratch_buffer.h>
-
-#include <libc-lock.h>
 #include <_itoa.h>
-
-/* Undefine the following line for the production version.  */
-/* #define NDEBUG 1 */
 #include <assert.h>
 
-/* Get some device information.  */
-#include <device-nrs.h>
-
 /*
  * This is a recursive-descent-style word expansion routine.
  */
@@ -812,61 +797,76 @@ parse_arith (char **word, size_t *word_length, size_t *max_length,
   return WRDE_SYNTAX;
 }
 
+#define DYNARRAY_STRUCT        strlist
+#define DYNARRAY_ELEMENT       char *
+#define DYNARRAY_PREFIX        strlist_
+/* Allocates about 512/1024 (32/64 bit) on stack.  */
+#define DYNARRAY_INITIAL_SIZE  128
+#include <malloc/dynarray-skeleton.c>
+
 /* Function called by child process in exec_comm() */
-static inline void
-__attribute__ ((always_inline))
-exec_comm_child (char *comm, int *fildes, int showerr, int noexec)
+static pid_t
+exec_comm_child (char *comm, int *fildes, bool showerr, bool noexec)
 {
-  const char *args[4] = { _PATH_BSHELL, "-c", comm, NULL };
+  pid_t pid = -1;
 
-  /* Execute the command, or just check syntax? */
-  if (noexec)
-    args[1] = "-nc";
+  /* Execute the command, or just check syntax?  */
+  const char *args[] = { _PATH_BSHELL, noexec ? "-nc" : "-c", comm, NULL };
+
+  posix_spawn_file_actions_t fa;
+  /* posix_spawn_file_actions_init does not fail.  */
+  __posix_spawn_file_actions_init (&fa);
 
-  /* Redirect output.  */
-  if (__glibc_likely (fildes[1] != STDOUT_FILENO))
+  /* Redirect output.  For check syntax only (noexec being true), exec_comm
+     explicits sets fildes[1] to -1, so check its value to avoid a failure in
+     __posix_spawn_file_actions_adddup2.  */
+  if (fildes[1] != -1)
     {
-      __dup2 (fildes[1], STDOUT_FILENO);
-      __close (fildes[1]);
+      if (__glibc_likely (fildes[1] != STDOUT_FILENO))
+	{
+	  if (__posix_spawn_file_actions_adddup2 (&fa, fildes[1],
+						  STDOUT_FILENO) != 0
+	      || __posix_spawn_file_actions_addclose (&fa, fildes[1]) != 0)
+	    goto out;
+	}
+      else
+	/* Reset the close-on-exec flag (if necessary).  */
+	if (__posix_spawn_file_actions_adddup2 (&fa, fildes[1], fildes[1])
+	    != 0)
+	  goto out;
     }
-  else
-    /* Reset the close-on-exec flag (if necessary).  */
-    __fcntl (fildes[1], F_SETFD, 0);
 
   /* Redirect stderr to /dev/null if we have to.  */
-  if (showerr == 0)
+  if (!showerr)
+    if (__posix_spawn_file_actions_addopen (&fa, STDERR_FILENO, _PATH_DEVNULL,
+					    O_WRONLY, 0) != 0)
+      goto out;
+
+  struct strlist newenv;
+  strlist_init (&newenv);
+
+  bool recreate_env = getenv ("IFS") != NULL;
+  if (recreate_env)
     {
-      struct stat64 st;
-      int fd;
-      __close (STDERR_FILENO);
-      fd = __open (_PATH_DEVNULL, O_WRONLY);
-      if (fd >= 0 && fd != STDERR_FILENO)
-	{
-	  __dup2 (fd, STDERR_FILENO);
-	  __close (fd);
-	}
-      /* Be paranoid.  Check that we actually opened the /dev/null
-	 device.  */
-      if (__builtin_expect (__fxstat64 (_STAT_VER, STDERR_FILENO, &st), 0) != 0
-	  || __builtin_expect (S_ISCHR (st.st_mode), 1) == 0
-#if defined DEV_NULL_MAJOR && defined DEV_NULL_MINOR
-	  || st.st_rdev != __gnu_dev_makedev (DEV_NULL_MAJOR, DEV_NULL_MINOR)
-#endif
-	  )
-	/* It's not the /dev/null device.  Stop right here.  The
-	   problem is: how do we stop?  We use _exit() with an
-	   hopefully unusual exit code.  */
-	_exit (90);
+      for (char **ep = __environ; *ep != NULL; ep++)
+	if (strncmp (*ep, "IFS=", strlen ("IFS=")) != 0)
+	  strlist_add (&newenv, *ep);
+      strlist_add (&newenv, NULL);
+      if (strlist_has_failed (&newenv))
+	goto out;
     }
 
-  /* Make sure the subshell doesn't field-split on our behalf. */
-  __unsetenv ("IFS");
+  /* pid is not set if posix_spawn fails, so it keep the original value
+     of -1.  */
+  __posix_spawn (&pid, _PATH_BSHELL, &fa, NULL, (char *const *) args,
+		 recreate_env ? strlist_begin (&newenv) : __environ);
 
-  __close (fildes[0]);
-  __execve (_PATH_BSHELL, (char *const *) args, __environ);
+  strlist_free (&newenv);
+
+out:
+  __posix_spawn_file_actions_destroy (&fa);
 
-  /* Bad.  What now?  */
-  abort ();
+  return pid;
 }
 
 /* Function to execute a command and retrieve the results */
@@ -884,13 +884,13 @@ exec_comm (char *comm, char **word, size_t *word_length, size_t *max_length,
   size_t maxnewlines = 0;
   char buffer[bufsize];
   pid_t pid;
-  int noexec = 0;
+  bool noexec = false;
 
   /* Do nothing if command substitution should not succeed.  */
   if (flags & WRDE_NOCMD)
     return WRDE_CMDSUB;
 
-  /* Don't fork() unless necessary */
+  /* Don't posix_spawn unless necessary */
   if (!comm || !*comm)
     return 0;
 
@@ -898,19 +898,15 @@ exec_comm (char *comm, char **word, size_t *word_length, size_t *max_length,
     return WRDE_NOSPACE;
 
  again:
-  if ((pid = __fork ()) < 0)
+  pid = exec_comm_child (comm, fildes, noexec ? false : flags & WRDE_SHOWERR,
+			 noexec);
+  if (pid < 0)
     {
-      /* Bad */
       __close (fildes[0]);
       __close (fildes[1]);
       return WRDE_NOSPACE;
     }
 
-  if (pid == 0)
-    exec_comm_child (comm, fildes, noexec ? 0 : flags & WRDE_SHOWERR, noexec);
-
-  /* Parent */
-
   /* If we are just testing the syntax, only wait.  */
   if (noexec)
     return (TEMP_FAILURE_RETRY (__waitpid (pid, &status, 0)) == pid
@@ -1091,7 +1087,7 @@ exec_comm (char *comm, char **word, size_t *word_length, size_t *max_length,
   /* Check for syntax error (re-execute but with "-n" flag) */
   if (buflen < 1 && status != 0)
     {
-      noexec = 1;
+      noexec = true;
       goto again;
     }
 
@@ -1143,26 +1139,9 @@ parse_comm (char **word, size_t *word_length, size_t *max_length,
 	      /* Go -- give script to the shell */
 	      if (comm)
 		{
-#ifdef __libc_ptf_call
-		  /* We do not want the exec_comm call to be cut short
-		     by a thread cancellation since cleanup is very
-		     ugly.  Therefore disable cancellation for
-		     now.  */
-		  // XXX Ideally we do want the thread being cancelable.
-		  // XXX If demand is there we'll change it.
-		  int state = PTHREAD_CANCEL_ENABLE;
-		  __libc_ptf_call (__pthread_setcancelstate,
-				   (PTHREAD_CANCEL_DISABLE, &state), 0);
-#endif
-
+		  /* posix_spawn already handles thread cancellation.  */
 		  error = exec_comm (comm, word, word_length, max_length,
 				     flags, pwordexp, ifs, ifs_white);
-
-#ifdef __libc_ptf_call
-		  __libc_ptf_call (__pthread_setcancelstate,
-				   (state, NULL), 0);
-#endif
-
 		  free (comm);
 		}