Background processes setting O_NONBLOCK on ttys

[Stephen McKay]

On Wednesday, 12th January 2005, Stephen McKay wrote:

>[Problems during Open Office compilation on FreeBSD 4.11-RC2]

>After some tracing, I have worked out that the tty is being alternately
>set to nonblocking and back to normal hundreds of times during the compilation
>of Open Office, and that's what is killing tail.

Turns out it's 1000's of times, not merely hundreds, but who's counting? :-)

And I know what does it now: absolutely every threaded program!  Scumbags!

Thanks to _thread_fd_table_init() called from _thread_init(), which seems
to get called pretty early on in a threaded program, every file descriptor
is set to non-blocking, including 0, 1 and 2.  Quite often these refer to
your tty, which means it gets set to non-blocking, even if the threaded
program has been invoked in the background.

I first noticed weird problems with vi some years ago which are consistent
with the tty (actually pty) being set to non-blocking, but have never found
a culprit until now.  But, having found the villian, what to do?

It is clearly wrong that foreground processes have to deal with the tty
spontaneously becoming non-blocking.  Every program, even "cat", would
have to be patched.  So, that's not an option.

I've patched my system so that you have to be in the foreground (ie match
the tty process group) in order to set non-blocking on a tty.  I'm pretty
pleased with this as it protects me from threaded programs, but there is
a drawback.  The unfortunate side effect is that all threaded programs
block if invoked with "&".  I can imagine others not liking this.

So, seriously, we could:

a) Rewrite file descriptor handling in libc_r so it does not set O_NONBLOCK
on tty file descriptors unless it is in the foreground.  I don't know how
hard this would be, or whether it even applies to -current with its profusion
of threading libraries.  I'll leave this to those who believe in threading.

b) Add new non-blocking read() and write() call variants and use them in
the threading library instead of setting O_NONBLOCK.

c) Make O_NONBLOCK be per file descriptor (like FD_CLOEXEC).  Thus,
descriptors produced from dup() (for example) would have their own O_NONBLOCK
flag, just as two descriptors from separate open() calls have today.

Option b) is probably a non-starter, though it would have been a better
way to do it if O_NONBLOCK wasn't already around (IMHO).

Option a) looks hard and still doesn't protect normal processes from all
others (including any old libc_r binaries out there).

Option c) is my clear choice, and I intend to implement this for my own
system before attempting to foist it upon everyone else.

Any thoughts?

By the way, I could find no documentation or standard that requires that
O_NONBLOCK apply in the way it does as opposed to per descriptor like I
am suggesting.

Stephen.

PS If anyone else wants to play with 4.11-RC2 with the background O_NONBLOCK 
prevention patch, here's what I have so far.

The patch to tty.c is straightforward.  The patch to kern_descrip.c fixes
what I think is a bug anyway: the file flags are updated even if FIONBIO
fails.

Index: tty.c
===================================================================
RCS file: /cvs/src/sys/kern/tty.c,v
retrieving revision 1.129.2.5
diff -u -r1.129.2.5 tty.c
--- tty.c	11 Mar 2002 01:32:31 -0000	1.129.2.5
+++ tty.c	12 Jan 2005 09:35:23 -0000
@@ -712,6 +712,7 @@
 
 	/* If the ioctl involves modification, hang if in the background. */
 	switch (cmd) {
+	case  FIONBIO:
 	case  TIOCCBRK:
 	case  TIOCCONS:
 	case  TIOCDRAIN:
Index: kern_descrip.c
===================================================================
RCS file: /cvs/src/sys/kern/kern_descrip.c,v
retrieving revision 1.81.2.19
diff -u -r1.81.2.19 kern_descrip.c
--- kern_descrip.c	28 Feb 2004 00:43:31 -0000	1.81.2.19
+++ kern_descrip.c	12 Jan 2005 11:56:27 -0000
@@ -218,7 +218,7 @@
 	register struct file *fp;
 	register char *pop;
 	struct vnode *vp;
-	int i, tmp, error, flg = F_POSIX;
+	int i, newflag, tmp, error, flg = F_POSIX;
 	struct flock fl;
 	u_int newmin;
 
@@ -252,22 +252,22 @@
 
 	case F_SETFL:
 		fhold(fp);
-		fp->f_flag &= ~FCNTLFLAGS;
-		fp->f_flag |= FFLAGS(uap->arg & ~O_ACCMODE) & FCNTLFLAGS;
-		tmp = fp->f_flag & FNONBLOCK;
+		newflag = fp->f_flag & ~FCNTLFLAGS;
+		newflag |= FFLAGS(uap->arg & ~O_ACCMODE) & FCNTLFLAGS;
+		tmp = newflag & FNONBLOCK;
 		error = fo_ioctl(fp, FIONBIO, (caddr_t)&tmp, p);
 		if (error) {
 			fdrop(fp, p);
 			return (error);
 		}
-		tmp = fp->f_flag & FASYNC;
+		tmp = newflag & FASYNC;
 		error = fo_ioctl(fp, FIOASYNC, (caddr_t)&tmp, p);
 		if (!error) {
 			fdrop(fp, p);
+			fp->f_flag = newflag;
 			return (0);
 		}
-		fp->f_flag &= ~FNONBLOCK;
-		tmp = 0;
+		tmp = fp->f_flag & FNONBLOCK;
 		(void)fo_ioctl(fp, FIONBIO, (caddr_t)&tmp, p);
 		fdrop(fp, p);
 		return (error);