From: Theodore Ts'o <tytso@athena.mit.edu> Subject: Re: Patch to fix vhangup() Date: 15 Feb 1993 22:05:51 -0500
From: bir7@leland.Stanford.EDU (Ross Biro)
Date: Mon, 15 Feb 93 21:10:01 GMT
I believe this patch opens a big security hole, and I would advise
against applying it. Basically it only gets rid of processes which
are accidently still attached to the tty, but doesn't deal with processes
which are intentionally trying to grab a tty and hide the fact that they
have done so.
Ross and I have talked this over (quite extensively) over private email,
and I believe I've managed to convince him that the security hole is not
much bigger than what was there before. Also, the current
implementation of vhangup() suffers from a serious defect, in that it
can cause unspecting programs to scribble over the wrong files.
tty_hangup() was getting rid of processes that were intentionally trying
to grab a tty; however, a process which was already blocked inside
read_chan() would allow one block of data to get through when it
shouldn't. Fortunately, however, tty_hangup() also wakes all of the
processes blocking on the tty, so the rogue process will probably not
get any characters, or at most one or two characters if it gets really
lucky.
Note that this bug was not introduced by my patch; it was always there,
and was caused because the check to see if a device was hung up was done
in the wrong place. Here is a patch to fix this:
*** 1.2 1993/02/15 21:49:34
--- tty_io.c 1993/02/15 22:03:44
***************
*** 651,656 ****
--- 651,660 ----
}
add_wait_queue(&tty->secondary.proc_list, &wait);
while (nr>0) {
+ if (hung_up(file)) {
+ file->f_flags &= ~O_NONBLOCK;
+ break; /* force read() to return 0 */
+ }
TTY_READ_FLUSH(tty);
if (tty->link)
TTY_WRITE_FLUSH(tty->link);
***************
*** 687,694 ****
TTY_WRITE_FLUSH(tty->link);
if (!EMPTY(&tty->secondary))
continue;
- if (hung_up(file))
- break;
current->state = TASK_INTERRUPTIBLE;
if (EMPTY(&tty->secondary))
schedule();
--- 691,696 ----
- Ted