Skip to content

L
linux
Switch branch/tag
  1. 29 Nov, 2010 4 commits
    • Jiri Slaby's avatar
      TTY: open/hangup race fixup · acfa747b
      Jiri Slaby authored 
      Like in the "TTY: don't allow reopen when ldisc is changing" patch,
this one fixes a TTY WARNING as described in the option 1) there:
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.

The fix is to introduce a new flag which we set during the unlocked
window and check it in tty_reopen too. The flag is TTY_HUPPING and is
cleared after TTY_HUPPED is set.

While at it, remove duplicate TTY_HUPPED set_bit. The one after
calling ops->hangup seems to be more correct. But anyway, we hold
tty_lock, so there should be no difference.

Also document the function it does that kind of crap.

Nicely reproducible with two forked children:
static void do_work(const char *tty)
{
	if (signal(SIGHUP, SIG_IGN) == SIG_ERR) exit(1);
	setsid();
	while (1) {
		int fd = open(tty, O_RDWR|O_NOCTTY);
		if (fd < 0) continue;
		if (ioctl(fd, TIOCSCTTY)) continue;
		if (vhangup()) continue;
		close(fd);
	}
	exit(0);
}
Signed-off-by: default avatarJiri Slaby <jslaby@suse.cz>
Reported-by: <Valdis.Kletnieks@vt.edu>
Reported-by: default avatarKyle 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>
      acfa747b
    • 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
    • Jiri Slaby's avatar
      NET: wan/x25, fix ldisc->open retval · 6a20bd45
      Jiri Slaby authored 
      We should never return positive values from ldisc->open, tty layer
doesn't (and never did) expect that.

If we do that, weird things like warnings in tty_ldisc_close happen.

Not sure if dev->base_addr is used somehow now.
Signed-off-by: default avatarJiri Slaby <jslaby@suse.cz>
Cc: Alan Cox <alan@linux.intel.com>
Cc: Andrew Hendry <andrew.hendry@gmail.com>
Cc: linux-x25@vger.kernel.org
Tested-by: default avatarSergey Lapin <slapin@ossfans.org>
Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@suse.de>
      6a20bd45
    • Jiri Slaby's avatar
      TTY: ldisc, fix open flag handling · 7f90cfc5
      Jiri Slaby authored 
      When a concrete ldisc open fails in tty_ldisc_open, we forget to clear
TTY_LDISC_OPEN. This causes a false warning on the next ldisc open:
WARNING: at drivers/char/tty_ldisc.c:445 tty_ldisc_open+0x26/0x38()
Hardware name: System Product Name
Modules linked in: ...
Pid: 5251, comm: a.out Tainted: G        W  2.6.32-5-686 #1
Call Trace:
 [<c1030321>] ? warn_slowpath_common+0x5e/0x8a
 [<c1030357>] ? warn_slowpath_null+0xa/0xc
 [<c119311c>] ? tty_ldisc_open+0x26/0x38
 [<c11936c5>] ? tty_set_ldisc+0x218/0x304
...

So clear the bit when failing...

Introduced in c65c9bc3 (tty: rewrite the ldisc locking) back in
2.6.31-rc1.
Signed-off-by: default avatarJiri Slaby <jslaby@suse.cz>
Cc: Alan Cox <alan@linux.intel.com>
Reported-by: default avatarSergey Lapin <slapin@ossfans.org>
Tested-by: default avatarSergey Lapin <slapin@ossfans.org>
Cc: stable <stable@kernel.org>
Signed-off-by: default avatarGreg Kroah-Hartman <gregkh@suse.de>
      7f90cfc5
  3. 16 Nov, 2010 2 commits
  5. 15 Nov, 2010 33 commits
  7. 14 Nov, 2010 1 commit
GitLab Nexedi Edition | About GitLab | About Nexedi | 沪ICP备2021021310号-2 | 沪ICP备2021021310号-7