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 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 thanks, -Jason --- linux/drivers/char/tty_io.c.bak +++ linux/drivers/char/tty_io.c @@ -1596,14 +1596,9 @@ static void release_dev(struct file * fi * each iteration we avoid any problems. */ while (1) { - /* Guard against races with tty->count changes elsewhere and - opens on /dev/tty */ - - down(&tty_sem); tty_closing = tty->count <= 1; o_tty_closing = o_tty && (o_tty->count <= (pty_master ? 1 : 0)); - up(&tty_sem); do_sleep = 0; if (tty_closing) { @@ -1640,7 +1635,6 @@ static void release_dev(struct file * fi * block, so it's safe to proceed with closing. */ - down(&tty_sem); if (pty_master) { if (--o_tty->count < 0) { printk(KERN_WARNING "release_dev: bad pty slave count " @@ -1654,7 +1648,6 @@ static void release_dev(struct file * fi tty->count, tty_name(tty, buf)); tty->count = 0; } - up(&tty_sem); /* * We've decremented tty->count, so we need to remove this file @@ -1844,9 +1837,10 @@ retry_open: } got_driver: retval = init_dev(driver, index, &tty); - up(&tty_sem); - if (retval) + if (retval) { + up(&tty_sem); return retval; + } filp->private_data = tty; file_move(filp, &tty->tty_files); @@ -1863,6 +1857,7 @@ got_driver: else retval = -ENODEV; } + up(&tty_sem); filp->f_flags = saved_flags; if (!retval && test_bit(TTY_EXCLUSIVE, &tty->flags) && !capable(CAP_SYS_ADMIN)) #include <sys/types.h> #include <sys/stat.h> #include <stdio.h> #include <stdlib.h> #include <unistd.h> #include <fcntl.h> #include <time.h> #include <pthread.h> #include <linux/fb.h> #include <linux/vt.h> #include <linux/kd.h> #define NTHREADS 300 void *thread_function(); int open_fail_num; int open_success; int main(int argc, char *argv[]) { int i, j; pthread_t thread_id[NTHREADS]; for(;;) { for(i=0; i < NTHREADS; i++) { pthread_create(&thread_id[i], NULL, &thread_function, NULL); } for(j=0; j < NTHREADS; j++) { pthread_join(thread_id[j], NULL); } printf("open failures: %i\n", open_fail_num); printf("open success: %i\n", open_success); } } void *thread_function() { int fd; time_t t; int val; int ret; fd = open("/dev/tty9", O_RDWR); val = 0; //call an ioctl ret = ioctl(fd, KDGETMODE, &val); if (ret != 0) { perror("ioctl error\n"); } if (fd < 0) { open_fail_num++; } else { open_success++; } /* just waste some random time */ t = (time((time_t *)0) &31L) << 6; while (t-- > 0) (void)time((time_t *)0); close(fd); } - To unsubscribe from this list: send the line "unsubscribe linux-kernel" in the body of a message to [EMAIL PROTECTED] More majordomo info at http://vger.kernel.org/majordomo-info.html Please read the FAQ at http://www.tux.org/lkml/
