Author: jilles
Date: Sun Nov 29 22:33:59 2009
New Revision: 199953
URL: http://svn.freebsd.org/changeset/base/199953

Log:
  Fix some cases where file descriptors from redirections leak to programs.
  
  - Redirecting fds that were not open before kept two copies of the
    redirected file.
      sh -c '{ :; } 7>/dev/null; fstat -p $$; true'
      (both fd 7 and 10 remained open)
  - File descriptors used to restore things after redirection were not
    set close-on-exec, instead they were explicitly closed before executing
    a program normally and before executing a shell procedure. The latter
    must remain but the former is replaced by close-on-exec.
      sh -c 'exec 7</; { exec fstat -p $$; } 7>/dev/null; true'
      (fd 10 remained open)
  
  The examples above are simpler than the testsuite because I do not want to
  use fstat or procstat in the testsuite.

Added:
  head/tools/regression/bin/sh/execution/redir1.0   (contents, props changed)
  head/tools/regression/bin/sh/execution/redir2.0   (contents, props changed)
Modified:
  head/bin/sh/eval.c
  head/bin/sh/redir.c

Modified: head/bin/sh/eval.c
==============================================================================
--- head/bin/sh/eval.c  Sun Nov 29 21:34:52 2009        (r199952)
+++ head/bin/sh/eval.c  Sun Nov 29 22:33:59 2009        (r199953)
@@ -883,7 +883,6 @@ cmddone:
 #ifdef DEBUG
                trputs("normal command:  ");  trargs(argv);
 #endif
-               clearredir();
                redirect(cmd->ncmd.redirect, 0);
                for (sp = varlist.list ; sp ; sp = sp->next)
                        setvareq(sp->text, VEXPORT|VSTACK);

Modified: head/bin/sh/redir.c
==============================================================================
--- head/bin/sh/redir.c Sun Nov 29 21:34:52 2009        (r199952)
+++ head/bin/sh/redir.c Sun Nov 29 22:33:59 2009        (r199953)
@@ -63,6 +63,7 @@ __FBSDID("$FreeBSD$");
 
 
 #define EMPTY -2               /* marks an unused slot in redirtab */
+#define CLOSED -1              /* fd was not open before redir */
 #define PIPESIZE 4096          /* amount of buffering in a pipe */
 
 
@@ -101,7 +102,6 @@ redirect(union node *redir, int flags)
        struct redirtab *sv = NULL;
        int i;
        int fd;
-       int try;
        char memory[10];        /* file descriptors to write to memory */
 
        for (i = 10 ; --i >= 0 ; )
@@ -116,38 +116,30 @@ redirect(union node *redir, int flags)
        }
        for (n = redir ; n ; n = n->nfile.next) {
                fd = n->nfile.fd;
-               try = 0;
                if ((n->nfile.type == NTOFD || n->nfile.type == NFROMFD) &&
                    n->ndup.dupfd == fd)
                        continue; /* redirect from/to same file descriptor */
 
                if ((flags & REDIR_PUSH) && sv->renamed[fd] == EMPTY) {
                        INTOFF;
-again:
                        if ((i = fcntl(fd, F_DUPFD, 10)) == -1) {
                                switch (errno) {
                                case EBADF:
-                                       if (!try) {
-                                               openredirect(n, memory);
-                                               try++;
-                                               goto again;
-                                       }
-                                       /* FALLTHROUGH*/
+                                       i = CLOSED;
+                                       break;
                                default:
                                        INTON;
                                        error("%d: %s", fd, strerror(errno));
                                        break;
                                }
-                       }
-                       if (!try) {
-                               sv->renamed[fd] = i;
-                       }
+                       } else
+                               (void)fcntl(i, F_SETFD, FD_CLOEXEC);
+                       sv->renamed[fd] = i;
                        INTON;
                }
                if (fd == 0)
                        fd0_redirected++;
-               if (!try)
-                       openredirect(n, memory);
+               openredirect(n, memory);
        }
        if (memory[1])
                out1 = &memout;

Added: head/tools/regression/bin/sh/execution/redir1.0
==============================================================================
--- /dev/null   00:00:00 1970   (empty, because file is newly added)
+++ head/tools/regression/bin/sh/execution/redir1.0     Sun Nov 29 22:33:59 
2009        (r199953)
@@ -0,0 +1,27 @@
+# $FreeBSD$
+trap ': $((brokenpipe+=1))' pipe
+
+P=${TMPDIR:-/tmp}
+cd $P
+T=$(mktemp -d sh-test.XXXXXX)
+cd $T
+
+brokenpipe=0
+mkfifo fifo1 fifo2
+read dummy >fifo2 <fifo1 &
+{
+       exec 4>fifo2
+} 3<fifo2 # Formerly, sh would keep fd 3 and a duplicate of it open.
+echo dummy >fifo1
+if [ $brokenpipe -ne 0 ]; then
+       rc=3
+fi
+wait
+echo dummy >&4
+if [ $brokenpipe -eq 1 ]; then
+       : ${rc:=0}
+fi
+
+rm fifo1 fifo2
+rmdir ${P}/${T}
+exit ${rc:-3}

Added: head/tools/regression/bin/sh/execution/redir2.0
==============================================================================
--- /dev/null   00:00:00 1970   (empty, because file is newly added)
+++ head/tools/regression/bin/sh/execution/redir2.0     Sun Nov 29 22:33:59 
2009        (r199953)
@@ -0,0 +1,29 @@
+# $FreeBSD$
+trap ': $((brokenpipe+=1))' pipe
+
+P=${TMPDIR:-/tmp}
+cd $P
+T=$(mktemp -d sh-test.XXXXXX)
+cd $T
+
+brokenpipe=0
+mkfifo fifo1 fifo2
+{
+       {
+               exec sh -c 'exec <fifo1; read dummy'
+       } 7<&- # fifo2 should be kept open, but not passed to programs
+       true
+} 7<fifo2 &
+
+exec 4>fifo2
+exec 3>fifo1
+echo dummy >&4
+if [ $brokenpipe -eq 1 ]; then
+       : ${rc:=0}
+fi
+echo dummy >&3
+wait
+
+rm fifo1 fifo2
+rmdir ${P}/${T}
+exit ${rc:-3}
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Reply via email to