42708: fix for process substitution.

Don't close associated file descriptors in the closem()
tidy up function as they should remain visible to external
processes.  Override if about to exit.

Unit test for the failing case: note this relies on the
existence of /proc/self/fd or equivalent.

5 files changed, 32 insertions, 10 deletions

diff --git a/ChangeLog b/ChangeLog
index 632a35717..535cf9f00 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,3 +1,10 @@
+2018-04-24  Peter Stephenson  <p.stephenson@samsung.com>
+
+	* 42708: Src/exec.c, Src/Modules/clone.c, Src/Modules/zpty.c,
+	Test/D03procsubst.ztst: don't close file descriptors for
+	process substitution on closem() tidy up unless we are exiting
+	as they need to be visible to external processes.
+
 2018-04-23  Peter Stephenson  <p.stephenson@samsung.com>
 
 	* 42705: Src/signals.c: another fix for 42630 --- also check
diff --git a/Src/Modules/clone.c b/Src/Modules/clone.c
index 930429248..ef6275dcf 100644
--- a/Src/Modules/clone.c
+++ b/Src/Modules/clone.c
@@ -69,7 +69,7 @@ bin_clone(char *nam, char **args, UNUSED(Options ops), UNUSED(int func))
 	dup2(ttyfd,2);
 	if (ttyfd > 2)
 	    close(ttyfd);
-	closem(0);
+	closem(FDT_UNUSED, 0);
 	close(coprocin);
 	close(coprocout);
 	/* Acquire a controlling terminal */
diff --git a/Src/Modules/zpty.c b/Src/Modules/zpty.c
index 1c93a1d02..2f83f7ce6 100644
--- a/Src/Modules/zpty.c
+++ b/Src/Modules/zpty.c
@@ -400,7 +400,7 @@ newptycmd(char *nam, char *pname, char **args, int echo, int nblock)
 	dup2(slave, 1);
 	dup2(slave, 2);
 
-	closem(0);
+	closem(FDT_UNUSED, 0);
 	close(slave);
 	close(master);
 	close(coprocin);
diff --git a/Src/exec.c b/Src/exec.c
index 1b622d56f..ddd9ee0ad 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -698,7 +698,7 @@ execute(LinkList args, int flags, int defpath)
      * Note that we don't close fd's attached to process substitution
      * here, which should be visible to external processes.
      */
-    closem(FDT_XTRACE);
+    closem(FDT_XTRACE, 0);
 #ifndef FD_CLOEXEC
     if (SHTTY != -1) {
 	close(SHTTY);
@@ -3943,7 +3943,7 @@ execcmd_exec(Estate state, Execcmd_params eparams,
 		LinkList assigns = (LinkList)0;
 		int postassigns = eparams->postassigns;
 		if (forked)
-		    closem(FDT_INTERNAL);
+		    closem(FDT_INTERNAL, 0);
 		if (postassigns) {
 		    Wordcode opc = state->pc;
 		    state->pc = eparams->assignspc;
@@ -4129,7 +4129,7 @@ execcmd_exec(Estate state, Execcmd_params eparams,
 		    if (errflag)
 			_exit(1);
 		}
-		closem(FDT_INTERNAL);
+		closem(FDT_INTERNAL, 0);
 		if (coprocin != -1) {
 		    zclose(coprocin);
 		    coprocin = -1;
@@ -4191,7 +4191,7 @@ execcmd_exec(Estate state, Execcmd_params eparams,
 	for (i = 0; i < 10; i++)
 	    if (fdtable[i] != FDT_UNUSED)
 		close(i);
-	closem(FDT_UNUSED);
+	closem(FDT_UNUSED, 1);
 	if (thisjob != -1)
 	    waitjobs();
 	_exit(lastval);
@@ -4367,16 +4367,24 @@ fixfds(int *save)
  *
  * Close any that are marked as used if "how" is FDT_UNUSED, else
  * close any with the value "how".
+ *
+ * If "all" is zero, we'll skip cases where we need the file
+ * descriptor to be visible externally.
  */
 
 /**/
 mod_export void
-closem(int how)
+closem(int how, int all)
 {
     int i;
 
     for (i = 10; i <= max_zsh_fd; i++)
 	if (fdtable[i] != FDT_UNUSED &&
+	    /*
+	     * Process substitution needs to be visible to user;
+	     * fd's are explicitly cleaned up by filelist handling.
+	     */
+	    (all || fdtable[i] != FDT_PROC_SUBST) &&
 	    (how == FDT_UNUSED || (fdtable[i] & FDT_TYPE_MASK) == how)) {
 	    if (i == SHTTY)
 		SHTTY = -1;
@@ -4859,7 +4867,7 @@ getproc(char *cmd, char **eptr)
 	    addproc(pid, NULL, 1, &bgtime);
 	return pnam;
     }
-    closem(FDT_UNUSED);
+    closem(FDT_UNUSED, 0);
     fd = open(pnam, out ? O_WRONLY | O_NOCTTY : O_RDONLY | O_NOCTTY);
     if (fd == -1) {
 	zerr("can't open %s: %e", pnam, errno);
@@ -4898,7 +4906,7 @@ getproc(char *cmd, char **eptr)
     }
     entersubsh(ESUB_ASYNC|ESUB_PGRP);
     redup(pipes[out], out);
-    closem(FDT_UNUSED);   /* this closes pipes[!out] as well */
+    closem(FDT_UNUSED, 0);   /* this closes pipes[!out] as well */
 #endif /* PATH_DEV_FD */
 
     cmdpush(CS_CMDSUBST);
@@ -4948,7 +4956,7 @@ getpipe(char *cmd, int nullexec)
     }
     entersubsh(ESUB_PGRP);
     redup(pipes[out], out);
-    closem(FDT_UNUSED);	/* this closes pipes[!out] as well */
+    closem(FDT_UNUSED, 0);	/* this closes pipes[!out] as well */
     cmdpush(CS_CMDSUBST);
     execode(prog, 0, 1, out ? "outsubst" : "insubst");
     cmdpop();
diff --git a/Test/D03procsubst.ztst b/Test/D03procsubst.ztst
index ca8d56ff5..1ef55821b 100644
--- a/Test/D03procsubst.ztst
+++ b/Test/D03procsubst.ztst
@@ -149,3 +149,10 @@
   fi
 0:proc subst fd in forked subshell closed in parent (external command)
 >1	1
+
+  procfunc() {
+    cat <(cat "$@")
+  }
+  procfunc <(echo argument)
+0:With /proc/self file descriptors must not be tidied up too early
+>argument