Re: fd used for saving redirected fds leaked to child processes

[Peter Stephenson <p.w.stephenson@xxxxxxxxxxxx>]

On Sun, 13 Aug 2017 17:12:07 +0100
Stephane Chazelas <stephane.chazelas@xxxxxxxxx> wrote:
> In:
> 
> mkfifo fifo
> zsh -c '{ echo GO > fifo & echo $!; } > pid; echo done' | cat
> 
> "cat" hangs until some process  open the fifo in read mode, even
> though that "echo GO > fifo" command was run in background and
> "done" is output straight away.
> 
> What we see is the child zsh process opening the "fifo" having
> a fd open on the pipe to cat:
> 
> zsh     28400 chazelas   12w  FIFO   0,10      0t0  125690 pipe 28399,cat,0r
> 
> I think that fd was the one that was dupped from stdout by its
> parent to /save/ stdout before doing the > pid redirection (so
> it can be used to restore stdout after the command group
> returns).

That explanation makes sense, in which case I think this fix should be
good enough.

pws

diff --git a/Src/exec.c b/Src/exec.c
index f339dd6..61ae5e8 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -947,6 +947,14 @@ hashcmd(char *arg0, char **pp)
 int
 forklevel;
 
+/* FDs saved for possible restoring, not needed in a subshell
+ * where we will never need to restore them.  Hence if we enter
+ * a subshell these will simply be closed unconditionally.
+ *
+ * A value >= 10 indicates a valid saved fd.
+ */
+static int saved_fds[10];
+
 /* Arguments to entersubsh() */
 enum {
     /* Subshell is to be run asynchronously (else synchronously) */
@@ -972,7 +980,7 @@ enum {
 static void
 entersubsh(int flags)
 {
-    int sig, monitor, job_control_ok;
+    int i, sig, monitor, job_control_ok;
 
     if (!(flags & ESUB_KEEPTRAP))
 	for (sig = 0; sig < SIGCOUNT; sig++)
@@ -1083,6 +1091,12 @@ entersubsh(int flags)
 	opts[MONITOR] = 0;
     opts[USEZLE] = 0;
     zleactive = 0;
+    for (i = 0; i != 10; i++) {
+	if (saved_fds[i] >= 10) {
+	    close(saved_fds[i]);
+	    saved_fds[i] = 0;
+	}
+    }
     if (flags & ESUB_PGRP)
 	clearjobtab(monitor);
     get_usage();
@@ -2318,6 +2332,7 @@ addfd(int forked, int *save, struct multio **mfds, int fd1, int fd2, int rflag,
 			return;
 		    }
 		    save[fd1] = fdN;
+		    saved_fds[fd1] = fdN;
 		}
 	    }
 	}
@@ -4249,9 +4264,12 @@ fixfds(int *save)
     int old_errno = errno;
     int i;
 
-    for (i = 0; i != 10; i++)
+    for (i = 0; i != 10; i++) {
 	if (save[i] != -2)
 	    redup(save[i], i);
+	if (save[i] >= 10)
+	    saved_fds[i] = 0;
+    }
     errno = old_errno;
 }