Patch to fix tty race condition

tytso@mit.edu
Sun, 11 Oct 1998 23:25:52 -0400


Hi Linus,

Enclosed please find a patch to fix a problem originally identified
by Bill Hawes. There's a race condition in the tty code which can cause
check_tty_count() to observe an inconsistent state, and printk a
warning. The patch fixes this bug, plus cleans up the tty code a little
bit (removed an erroneous comment and an extraneous blank line).

I'd appreciate it if you could apply the enclosed patch to the
2.1 kernel sources. Thanks!!

Here's Bill's original commentary, explianing the problem:

>I did some further review of the tty release_dev and file closing paths,
>and have found a small problem in the handling of filp->private_data
>that could (rarely) trigger one of the tty count warning messages. It
>involves a subtle race that may result in the filp->private_data for a
>closed file being counted as an active use of the tty.
>
>The filp->private_data field defines the association between files and
>ttys, with one tty possibly having many associated files. Since the call
>to release_dev() indicates the last use of a file, the association with
>the tty is logically complete at this time, and private_data should be
>cleared. However, the current code only clears private_data when the
>_tty_ is closed, which happens only when the last of the multiple files
>is closed.
>
>Normally this isn't a problem, as the released file is soon removed from
>the inuse_filp list. But an examination of the closing path shows calls
>in the following sequence:
>
> close_fp
> fput
> __fput
> release (resolves to tty release_dev)
> dput <-- could block
> remove_filp <-- filp removed from inuse_filp
> insert_file_free
>
>After the call to release_dev the file will still be on the inuse_filp
>list with private_data pointing to the tty, even though the tty->count
>for the file has been decremented. The race occurs because the call to
>dput() could block, thereby allowing a call to check_tty_count to
>observe the inconsistent state.

- Ted

Patch generated: on Sun Oct 11 20:05:31 EDT 1998 by tytso@rsts-11.mit.edu
against Linux version 2.1.125

===================================================================
RCS file: drivers/char/RCS/tty_io.c,v
retrieving revision 1.1
diff -u -r1.1 drivers/char/tty_io.c
--- drivers/char/tty_io.c 1998/10/11 23:21:38 1.1
+++ drivers/char/tty_io.c 1998/10/11 23:39:16
@@ -1132,7 +1132,6 @@
* both sides, and we've completed the last operation that could
* block, so it's safe to proceed with closing.
*/
-
if (pty_master) {
if (--o_tty->count < 0) {
printk("release_dev: bad pty slave count (%d) for %s\n",
@@ -1147,6 +1146,16 @@
}

/*
+ * We've decremented tty->count, so we should zero out
+ * filp->private_data, to break the link between the tty and
+ * the file descriptor. Otherwise if close_fp() blocks before
+ * the the file descriptor is removed from the inuse_filp
+ * list, check_tty_count() could observe a discrepancy and
+ * printk a warning message to the user.
+ */
+ filp->private_data = 0;
+
+ /*
* Perform some housekeeping before deciding whether to return.
*
* Set the TTY_CLOSING flag if this was the last open. In the
@@ -1180,7 +1189,6 @@
/* check whether both sides are closing ... */
if (!tty_closing || (o_tty && !o_tty_closing))
return;
- filp->private_data = 0;

#ifdef TTY_DEBUG_HANGUP
printk("freeing tty structure...");
@@ -1293,7 +1301,6 @@
return retval;

#ifdef CONFIG_UNIX98_PTYS
- /* N.B. this error exit may leave filp->f_flags with O_NONBLOCK set */
init_dev_done:
#endif
filp->private_data = tty;

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.rutgers.edu
Please read the FAQ at http://www.tux.org/lkml/