redir: Handle nested exec within REALLY_CLOSED redirection

The value of REALLY_CLOSED is used to avoid an unnecessary close(2)
call when restoring redirections.  However, as it stands it can
remove a close(2) call that's actually needed.  This happens when
an enclosed exec(1) command leaves an open file descriptor behind.

This patch fixes this by replacing REALLY_CLOSED with closed_redirs
to track the current status of redirected file descriptors and
leaving redirlist to only handle the previous state of redirected
file descriptors.

Reported-by: Martijn Dekker <martijn@inlv.org>
Fixes: ce0f1900d869 ("[REDIR] Fix redirect restore on saved file...")
Signed-off-by: Herbert Xu <herbert@gondor.apana.org.au>

1 files changed, 30 insertions, 10 deletions

diff --git a/src/redir.c b/src/redir.c
index e67cc0a..6c81dd0 100644
--- a/src/redir.c
+++ b/src/redir.c
@@ -57,7 +57,6 @@
 #include "error.h"
 
 
-#define REALLY_CLOSED -3	/* fd that was closed and still is */
 #define EMPTY -2		/* marks an unused slot in redirtab */
 #define CLOSED -1		/* fd opened for redir needs to be closed */
 
@@ -77,6 +76,9 @@ struct redirtab {
 
 MKINIT struct redirtab *redirlist;
 
+/* Bit map of currently closed file descriptors. */
+static unsigned closed_redirs;
+
 STATIC int openredirect(union node *);
 #ifdef notyet
 STATIC void dupredirect(union node *, int, char[10]);
@@ -86,6 +88,20 @@ STATIC void dupredirect(union node *, int);
 STATIC int openhere(union node *);
 
 
+static unsigned update_closed_redirs(int fd, int nfd)
+{
+	unsigned val = closed_redirs;
+	unsigned bit = 1 << fd;
+
+	if (nfd >= 0)
+		closed_redirs &= ~bit;
+	else
+		closed_redirs |= bit;
+
+	return val & bit;
+}
+
+
 /*
  * Process a list of redirection commands.  If the REDIR_PUSH flag is set,
  * old file descriptors are stashed away so that the redirection can be
@@ -125,21 +141,21 @@ redirect(union node *redir, int flags)
 		fd = n->nfile.fd;
 
 		if (sv) {
+			int closed;
+
 			p = &sv->renamed[fd];
 			i = *p;
 
+			closed = update_closed_redirs(fd, newfd);
+
 			if (likely(i == EMPTY)) {
 				i = CLOSED;
-				if (fd != newfd) {
+				if (fd != newfd && !closed) {
 					i = savefd(fd, fd);
 					fd = -1;
 				}
 			}
 
-			if (i == newfd)
-				/* Can only happen if i == newfd == CLOSED */
-				i = REALLY_CLOSED;
-
 			*p = i;
 		}
 
@@ -346,14 +362,18 @@ popredir(int drop)
 	INTOFF;
 	rp = redirlist;
 	for (i = 0 ; i < 10 ; i++) {
+		int closed;
+
+		if (rp->renamed[i] == EMPTY)
+			continue;
+
+		closed = drop ? 1 : update_closed_redirs(i, rp->renamed[i]);
+
 		switch (rp->renamed[i]) {
 		case CLOSED:
-			if (!drop)
+			if (!closed)
 				close(i);
 			break;
-		case EMPTY:
-		case REALLY_CLOSED:
-			break;
 		default:
 			if (!drop)
 				dup2(rp->renamed[i], i);