lkml.org 
[lkml]   [2005]   [Apr]   [20]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
Patch in this message
/
Date
From
Subject[PATCH] tty races

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 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-20 19:50    [W:0.085 / U:2.308 seconds]
©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site