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

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

On Mon, 14 Aug 2017 07:49:52 -0700
Bart Schaefer <schaefer@xxxxxxxxxxxxxxxx> wrote:
> On Mon, Aug 14, 2017 at 2:09 AM, Peter Stephenson
> <p.stephenson@xxxxxxxxxxx> wrote:
> >
> > We need to expose the entire hierarchy for this particular case, but
> > that looks like a stack or a linked list, which seems a little
> > heavyweight for this case.
> 
> Another flag in the fdtable array?

This does that --- this looks easy.  It didn't seem convenient to
optimise, since once one set of saved fds is restored, exposing any in
an enclosing scope, there's no way of working out which the new highest
number is without an exhaustive search.

pws

diff --git a/Src/exec.c b/Src/exec.c
index f339dd6..9996dff 100644
--- a/Src/exec.c
+++ b/Src/exec.c
@@ -972,7 +972,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 +1083,14 @@ entersubsh(int flags)
 	opts[MONITOR] = 0;
     opts[USEZLE] = 0;
     zleactive = 0;
+    /*
+     * If we've saved fd's for later restoring, we're never going
+     * to restore them now, so just close them.
+     */
+    for (i = 10; i <= max_zsh_fd; i++) {
+	if (fdtable[i] & FDT_SAVED_MASK)
+	    zclose(i);
+    }
     if (flags & ESUB_PGRP)
 	clearjobtab(monitor);
     get_usage();
@@ -2318,6 +2326,9 @@ addfd(int forked, int *save, struct multio **mfds, int fd1, int fd2, int rflag,
 			return;
 		    }
 		    save[fd1] = fdN;
+		    DPUTS(fdtable[fdN] != FDT_INTERNAL,
+			  "Saved file descriptor not marked as internal");
+		    fdtable[fdN] |= FDT_SAVED_MASK;
 		}
 	    }
 	}
@@ -3575,7 +3586,8 @@ execcmd_exec(Estate state, Execcmd_params eparams,
 			    }
 			    if (!bad && fn->fd1 <= max_zsh_fd) {
 				if (fn->fd1 >= 10 &&
-				    fdtable[fn->fd1] == FDT_INTERNAL)
+				    (fdtable[fn->fd1] & FDT_TYPE_MASK) ==
+				    FDT_INTERNAL)
 				    bad = 3;
 			    }
 			}
@@ -4270,7 +4282,7 @@ closem(int how)
 
     for (i = 10; i <= max_zsh_fd; i++)
 	if (fdtable[i] != FDT_UNUSED &&
-	    (how == FDT_UNUSED || fdtable[i] == how)) {
+	    (how == FDT_UNUSED || (fdtable[i] & FDT_TYPE_MASK) == how)) {
 	    if (i == SHTTY)
 		SHTTY = -1;
 	    zclose(i);
diff --git a/Src/zsh.h b/Src/zsh.h
index ccd11db..f42ada7 100644
--- a/Src/zsh.h
+++ b/Src/zsh.h
@@ -456,6 +456,18 @@ enum {
 #define FDT_PROC_SUBST		7
 #endif
 
+/*
+ * Mask to get the basic FDT type.
+ */
+#define FDT_TYPE_MASK		15
+
+/*
+ * Bit flag that fd is saved for later restoration.
+ * Currently this is only use with FDT_INTERNAL.  We use this fact so as
+ * not to have to mask checks against other types.
+ */
+#define FDT_SAVED_MASK		16
+
 /* Flags for input stack */
 #define INP_FREE      (1<<0)	/* current buffer can be free'd            */
 #define INP_ALIAS     (1<<1)	/* expanding alias or history              */