![]() | |||||||||||||
Messages in this thread |
Jason Baron <jbaron@redhat.com> wrote: > > There are a couple of tty race conditions, which lead to inconsistent tty > reference counting and tty layer oopses. > > The first is a tty_open vs. tty_close race in drivers/char/tty.io.c. > Basically, from the time that the tty->count is deemed to be 1 and that we > are going to free it to the time that TTY_CLOSING bit is set, needs to be > atomic with respect to the manipulation of tty->count in init_dev(). This > atomicity was previously guarded by the BKL. However, this is no longer > true with the addition of a down() call in the middle of the > release_dev()'s atomic path. So either the down() needs to be moved > outside the atomic patch or dropped. I would vote for simply dropping it > as i don't see why it is necessary. The release_dev() changes looks very fishy to me. It _removes_ locking. If that fixes the testcase then one of two things is happening: a) we have lock_kernel() coverage and the down()'s sleeping breaks the lock_kenrel() coverage or b) we don't have lock_kernel() coverage, but removing the down() just alters the timing and makes the race less probable. I think it's b). lock_kernel() coverage in there is very incomplete on the open() side. I think it would be better to _increase_ the tty_sem coverage in release_dev() and to make sure that all callers of init_dev() are using tty_sem (they are). One approach would be to require that all callers of release_dev() hold tty_sem, and make release_dev() drop and reacquire tty_sem in those cases where release_dev() needs to go to sleep when waiting for other threads of control to reelase the tty's resources. > The second race is tty_open vs. tty_open. This race I've seen when the > virtual console is the tty driver. In con_open(), vc_allocate() is called > if the tty->count is 1. However, this check of the tty->count is not > guarded by the 'tty_sem'. Thus, it is possible for con_open(), to never > see the tty->count as 1, and thus never call vc_allocate(). This leads to > a NULL filp->private_data, and an oops. > > The test case below reproduces these problems, and the patch fixes it. The > test case uses /dev/tty9, which is generally restricted to root for > open(). It may be able to exploit these races using pseudo terminals, > although i wasn't able to. A previous report of this issue, with an oops > trace was: http://www.ussg.iu.edu/hypermail/linux/kernel/0503.2/0017.html So you've extended tty_sem coverage over the tty driver's ->open method. I guess that's OK. - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/ | ||||||||||||
| Last update: 2005-04-26 06:26 [from the cache] ©2003-2008 | |||||||||||||