[src/trunk]: src/bin/sh Fix an ordering error in the previous (and even earli...

[kre <kre%NetBSD.org@localhost>]

details:   https://anonhg.NetBSD.org/src/rev/f537d39d17c2
branches:  trunk
changeset: 986259:f537d39d17c2
user:      kre <kre%NetBSD.org@localhost>
date:      Wed Sep 15 20:21:47 2021 +0000

description:
Fix an ordering error in the previous (and even earlier, going back
a way, but made more serious with the recent changes).

The n>&n operation (more or less a no-op, except it clears CLOEXEC)
should precede almost everything else - and simply be made to fail if
an attempt is made to apply it to a sh internal fd.

We were renumbering the internal fd (the n> part considered first)
which was dumb, but OK, before, but now rejecting the operation
(the >&n) part when n should not be visible to the script.  That
made something of a mess (and could lead to the shell believing its
job control tty was at a fd it never got moved to).

Do things in the correct order, and simply fail that case for internal
fds (for every other n>xxx for any xxx sh simply renumbers its internal fd
n to some other fd before attempting the operation, even n>&- ... those are
all fine).

[In all the above the '>' is used in place of any redirect operator].

diffstat:

 bin/sh/redir.c |  26 +++++++++++++++-----------
 1 files changed, 15 insertions(+), 11 deletions(-)

diffs (61 lines):

diff -r 48b0d0390089 -r f537d39d17c2 bin/sh/redir.c
--- a/bin/sh/redir.c    Wed Sep 15 18:30:57 2021 +0000
+++ b/bin/sh/redir.c    Wed Sep 15 20:21:47 2021 +0000
@@ -1,4 +1,4 @@
-/*     $NetBSD: redir.c,v 1.68 2021/09/15 18:29:45 kre Exp $   */
+/*     $NetBSD: redir.c,v 1.69 2021/09/15 20:21:47 kre Exp $   */
 
 /*-
  * Copyright (c) 1991, 1993
@@ -37,7 +37,7 @@
 #if 0
 static char sccsid[] = "@(#)redir.c    8.2 (Berkeley) 5/4/95";
 #else
-__RCSID("$NetBSD: redir.c,v 1.68 2021/09/15 18:29:45 kre Exp $");
+__RCSID("$NetBSD: redir.c,v 1.69 2021/09/15 20:21:47 kre Exp $");
 #endif
 #endif /* not lint */
 
@@ -245,6 +245,18 @@
                    fd, max_user_fd, user_fd_limit));
                if (fd < user_fd_limit && fd > max_user_fd)
                        max_user_fd = fd;
+               if ((n->nfile.type == NTOFD || n->nfile.type == NFROMFD) &&
+                   n->ndup.dupfd == fd) {
+                       VTRACE(DBG_REDIR, ("!cloexec\n"));
+                       if (sh_fd(fd) != NULL ||
+                           saved_redirected_fd(fd) != NULL)
+                               error("fd %d: %s", fd, strerror(EBADF));
+                       /* redirect from/to same file descriptor */
+                       /* make sure it stays open */
+                       if (fcntl(fd, F_SETFD, 0) < 0)
+                               error("fd %d: %s", fd, strerror(errno));
+                       continue;
+               }
                if ((renamed = saved_redirected_fd(fd)) != NULL) {
                        int to = pick_new_fd(fd);
 
@@ -255,15 +267,6 @@
                                (void)close(fd);
                }
                renumber_sh_fd(sh_fd(fd));
-               if ((n->nfile.type == NTOFD || n->nfile.type == NFROMFD) &&
-                   n->ndup.dupfd == fd) {
-                       /* redirect from/to same file descriptor */
-                       /* make sure it stays open */
-                       if (fcntl(fd, F_SETFD, 0) < 0)
-                               error("fd %d: %s", fd, strerror(errno));
-                       VTRACE(DBG_REDIR, ("!cloexec\n"));
-                       continue;
-               }
 
                if ((flags & REDIR_PUSH) && !is_renamed(sv->renamed, fd)) {
                        int bigfd;
@@ -421,6 +424,7 @@
                if (copyfd(f, fd, cloexec) < 0) {
                        int e = errno;
 
+                       VTRACE(DBG_REDIR, (" failed: %s\n", strerror(e)));
                        close(f);
                        error("redirect reassignment (fd %d) failed: %s", fd,
                            strerror(e));