• Jiri Slaby's avatar
    TTY: don't allow reopen when ldisc is changing · e2efafbf
    Jiri Slaby authored
    There are many WARNINGs like the following reported nowadays:
    WARNING: at drivers/tty/tty_io.c:1331 tty_open+0x2a2/0x49a()
    Hardware name: Latitude E6500
    Modules linked in:
    Pid: 1207, comm: plymouthd Not tainted 2.6.37-rc3-mmotm1123 #3
    Call Trace:
     [<ffffffff8103b189>] warn_slowpath_common+0x80/0x98
     [<ffffffff8103b1b6>] warn_slowpath_null+0x15/0x17
     [<ffffffff8128a3ab>] tty_open+0x2a2/0x49a
     [<ffffffff810fd53f>] chrdev_open+0x11d/0x146
    ...
    
    This means tty_reopen is called without TTY_LDISC set. For further
    considerations, note tty_lock is held in tty_open. TTY_LDISC is cleared in:
    1) __tty_hangup from tty_ldisc_hangup to tty_ldisc_enable. During this
    section tty_lock is held. However tty_lock is temporarily dropped in
    the middle of the function by tty_ldisc_hangup.
    
    2) tty_release via tty_ldisc_release till the end of tty existence. If
    tty->count <= 1, tty_lock is taken, TTY_CLOSING bit set and then
    tty_ldisc_release called. tty_reopen checks TTY_CLOSING before checking
    TTY_LDISC.
    
    3) tty_set_ldisc from tty_ldisc_halt to tty_ldisc_enable. We:
       * take tty_lock, set TTY_LDISC_CHANGING, put tty_lock
       * call tty_ldisc_halt (clear TTY_LDISC), tty_lock is _not_ held
       * do some other work
       * take tty_lock, call tty_ldisc_enable (set TTY_LDISC), put
         tty_lock
    
    I cannot see how 2) can be a problem, as there I see no race. OTOH, 1)
    and 3) can happen without problems. This patch the case 3) by checking
    TTY_LDISC_CHANGING along with TTY_CLOSING in tty_reopen. 1) will be
    fixed in the following patch.
    
    Nicely reproducible with two processes:
    while (1) {
    	fd = open("/dev/ttyS1", O_RDWR);
    	if (fd < 0) {
    		warn("open");
    		continue;
    	}
    	close(fd);
    }
    --------
    while (1) {
            fd = open("/dev/ttyS1", O_RDWR);
            ld1 = 0; ld2 = 2;
            while (1) {
                    ioctl(fd, TIOCSETD, &ld1);
                    ioctl(fd, TIOCSETD, &ld2);
            }
            close(fd);
    }
    Signed-off-by: default avatarJiri Slaby <jslaby@suse.cz>
    Reported-by: <Valdis.Kletnieks@vt.edu>
    Cc: Kyle McMartin <kyle@mcmartin.ca>
    Cc: Alan Cox <alan@lxorguk.ukuu.org.uk>
    Cc: stable <stable@kernel.org>
    Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@suse.de>
    e2efafbf
tty_io.c 80.1 KB