From efffe9c8536bf9ee28f2f381bd285824bedcdbcd Mon Sep 17 00:00:00 2001 From: Andrew Morton <akpm@osdl.org> Date: Sun, 11 Apr 2004 22:40:55 -0700 Subject: [PATCH] [PATCH] Fix VT open/close race The race is that con_close() can sleep, and drops the BKL while tty->count==1. But another thread can come into init_dev() and will take a new ref against the tty and start using it. But con_close() doesn't notice that new ref and proceeds to null out tty->driver_data while someone else is using the resurrected tty. So the patch serialises con_close() against init_dev() with tty_sem. Here's a test app which reproduced the oops instantly on 2-way. It realy needs to be run against all tty-capable devices. /* * Run this against a tty which nobody currently has open, such as /dev/tty9 */ #include <stdio.h> #include <stdlib.h> #include <unistd.h> #include <fcntl.h> #include <sys/ioctl.h> #include <linux/kd.h> void doit(char *filename) { int fd,x; fd = open(filename, O_RDWR); if (fd < 0) { perror("open"); exit(1); } ioctl(fd, KDKBDREP, &x); close(fd); } main(int argc, char *argv[]) { char *filename = argv[1]; for ( ; ; ) doit(filename); } --- drivers/char/tty_io.c | 2 +- drivers/char/vt.c | 14 ++++++++++++++ include/linux/tty.h | 3 +++ 3 files changed, 18 insertions(+), 1 deletion(-) diff --git a/drivers/char/tty_io.c b/drivers/char/tty_io.c index 6bb5ae7e41a5..0ba52078f637 100644 --- a/drivers/char/tty_io.c +++ b/drivers/char/tty_io.c @@ -123,7 +123,7 @@ LIST_HEAD(tty_drivers); /* linked list of tty drivers */ struct tty_ldisc ldiscs[NR_LDISCS]; /* line disc dispatch table */ /* Semaphore to protect creating and releasing a tty */ -static DECLARE_MUTEX(tty_sem); +DECLARE_MUTEX(tty_sem); #ifdef CONFIG_UNIX98_PTYS extern struct tty_driver *ptm_driver; /* Unix98 pty masters; for /dev/ptmx */ diff --git a/drivers/char/vt.c b/drivers/char/vt.c index a5ddfc5ac9c1..2febed52e19f 100644 --- a/drivers/char/vt.c +++ b/drivers/char/vt.c @@ -2480,8 +2480,16 @@ static int con_open(struct tty_struct *tty, struct file *filp) return ret; } +/* + * We take tty_sem in here to prevent another thread from coming in via init_dev + * and taking a ref against the tty while we're in the process of forgetting + * about it and cleaning things up. + * + * This is because vcs_remove_devfs() can sleep and will drop the BKL. + */ static void con_close(struct tty_struct *tty, struct file *filp) { + down(&tty_sem); acquire_console_sem(); if (tty && tty->count == 1) { struct vt_struct *vt; @@ -2492,9 +2500,15 @@ static void con_close(struct tty_struct *tty, struct file *filp) tty->driver_data = 0; release_console_sem(); vcs_remove_devfs(tty); + up(&tty_sem); + /* + * tty_sem is released, but we still hold BKL, so there is + * still exclusion against init_dev() + */ return; } release_console_sem(); + up(&tty_sem); } static void vc_init(unsigned int currcons, unsigned int rows, diff --git a/include/linux/tty.h b/include/linux/tty.h index fbcc401e8b28..6e61f3b27157 100644 --- a/include/linux/tty.h +++ b/include/linux/tty.h @@ -363,6 +363,9 @@ extern void tty_flip_buffer_push(struct tty_struct *tty); extern int tty_get_baud_rate(struct tty_struct *tty); extern int tty_termios_baud_rate(struct termios *termios); +struct semaphore; +extern struct semaphore tty_sem; + /* n_tty.c */ extern struct tty_ldisc tty_ldisc_N_TTY; -- 2.30.9