Bug 47136

I found, that sh uses fd 3 for the controlling terminal. /bin/sh opens
/dev/tty and stores the resulting file descriptor in ttyfd:

jobs.c:
122:    if (on) {
123:            if (ttyfd != -1)
124:                    close(ttyfd);
125:            if ((ttyfd = open(_PATH_TTY, O_RDWR)) < 0) {
126:                    i = 0;
127:                    while (i <= 2 && !isatty(i))
128:                            i++;
129:                    if (i > 2 || (ttyfd = dup(i)) < 0)
130:                            goto out;
131:            }

ttyfd is needed for calls to tcsetpgrp.

To prevent conflicts with user-fd, ttyfd should be moved to a higher fd
like bash2 does:

jobs.c:
2911:      /* Get our controlling terminal.  If job_control is set, or
2912:    interactive is set, then this is an interactive shell no
2913:    matter where fd 2 is directed. */
2914:      shell_tty = dup (fileno (stderr));   /* fd 2 */
2915:
2916:      shell_tty = move_to_high_fd (shell_tty, 1, -1);
2917:

here's a patch proposal.

Sorry for sending mails twice, addresses were wrong. :/

Friedemann




--- /usr/src/bin/sh/jobs.c	Wed Jan 22 02:30:07 2003
+++ jobs.c	Sun Jan 26 19:05:53 2003
@@ -76,6 +76,7 @@
 #include "mystring.h"


+
 struct job *jobtab;		/* array of jobs */
 int njobs;			/* size of array */
 MKINIT pid_t backgndpid = -1;	/* pid of last background process */
@@ -105,6 +106,9 @@
 STATIC void showjob(struct job *, pid_t, int, int);


+int move_to_high_fd __P((int, int, int));
+
+
 /*
  * Turn job control on and off.
  */
@@ -129,6 +133,8 @@
 			if (i > 2 || (ttyfd = dup(i)) < 0)
 				goto out;
 		}
+		ttyfd = move_to_high_fd (ttyfd, 1, -1);
+
 		if (fcntl(ttyfd, F_SETFD, FD_CLOEXEC) < 0) {
 			close(ttyfd);
 			ttyfd = -1;
@@ -1227,4 +1233,48 @@
 		}
 	}
 	cmdnextc = q;
+}
+
+
+
+#define HIGH_FD_MAX	256
+
+/* Move FD to a number close to the maximum number of file descriptors
+   allowed in the shell process, to avoid the user stepping on it with
+   redirection and causing us extra work.  If CHECK_NEW is non-zero,
+   we check whether or not the file descriptors are in use before
+   duplicating FD onto them.  MAXFD says where to start checking the
+   file descriptors.  If it's less than 20, we get the maximum value
+   available from getdtablesize(2). */
+int
+move_to_high_fd (fd, check_new, maxfd)
+	int fd, check_new, maxfd;
+{
+	int script_fd, nfds, ignore;
+
+	if (maxfd < 20)
+	{
+		nfds = getdtablesize ();
+		if (nfds <= 0)
+			nfds = 20;
+		if (nfds > HIGH_FD_MAX)
+			nfds = HIGH_FD_MAX;		/* reasonable maximum */
+	}
+	else
+		nfds = maxfd;
+
+	for (nfds--; check_new && nfds > 3; nfds--)
+		if (fcntl (nfds, F_GETFD, &ignore) == -1)
+			break;
+
+	if (nfds > 3 && fd != nfds && (script_fd = dup2 (fd, nfds)) != -1)
+	{
+		if (check_new == 0 || fd != 2)	/* don't close stderr */
+			close (fd);
+		return (script_fd);
+	}
+
+	/* OK, we didn't find one less than our artificial maximum; return the
+	   original file descriptor. */
+	return (fd);
 }

Comment 2 Tim Robbins 2003-01-27 06:33:37 UTC

Responsible Changed
From-To: freebsd-bugs->tjr

I'll fix this.

Comment 3 Tim Robbins 2003-01-27 07:41:22 UTC

State Changed
From-To: open->patched

Fixed in -current, change will be MFC'd after one week. 
Thanks for the report.

Comment 4 Tim Robbins 2003-02-04 02:40:26 UTC

State Changed
From-To: patched->closed

Fixed in RELENG_4; thanks for the report.