Commit ce8b13c9 authored by Andrew Morton's avatar Andrew Morton Committed by Linus Torvalds

[PATCH] drivers/char/vt possible race

From: Benjamin Herrenschmidt <benh@kernel.crashing.org>

I falled again on the crash in con_do_write() with driver->data beeing
NULL.  It happens during boot, when userland is playing open/close games
with tty's, I was intentionally typing keys like mad during boot trying to
trigger another problem when this one poped up.

Looking at the code, I'm not sure how protected we are by the above (tty)
layer, paulus told me to not rely on anything like locking coming from
there, so I decided to extend the scope of the console semaphore one more
bit to cover races between calls to con_open, con_close and con_write.
Note that in con_do_write, I intentionally drop the semaphore to avoid
keeping it held when waiting on the local buffer, and I added some sanity
checks on tty->driver_data with some printk's in case we still have an open
race by the tty layer.  At least, now, the couple vc_allocated &
tty->driver_data should be protected though.
parent 9b6722ed
...@@ -1883,14 +1883,24 @@ static int do_con_write(struct tty_struct * tty, int from_user, ...@@ -1883,14 +1883,24 @@ static int do_con_write(struct tty_struct * tty, int from_user,
int c, tc, ok, n = 0, draw_x = -1; int c, tc, ok, n = 0, draw_x = -1;
unsigned int currcons; unsigned int currcons;
unsigned long draw_from = 0, draw_to = 0; unsigned long draw_from = 0, draw_to = 0;
struct vt_struct *vt = (struct vt_struct *)tty->driver_data; struct vt_struct *vt;
u16 himask, charmask; u16 himask, charmask;
const unsigned char *orig_buf = NULL; const unsigned char *orig_buf = NULL;
int orig_count; int orig_count;
if (in_interrupt()) if (in_interrupt())
return count; return count;
might_sleep();
acquire_console_sem();
vt = (struct vt_struct *)tty->driver_data;
if (vt == NULL) {
printk(KERN_ERR "vt: argh, driver_data is NULL !\n");
release_console_sem();
return 0;
}
currcons = vt->vc_num; currcons = vt->vc_num;
if (!vc_cons_allocated(currcons)) { if (!vc_cons_allocated(currcons)) {
/* could this happen? */ /* could this happen? */
...@@ -1899,13 +1909,16 @@ static int do_con_write(struct tty_struct * tty, int from_user, ...@@ -1899,13 +1909,16 @@ static int do_con_write(struct tty_struct * tty, int from_user,
error = 1; error = 1;
printk("con_write: tty %d not allocated\n", currcons+1); printk("con_write: tty %d not allocated\n", currcons+1);
} }
release_console_sem();
return 0; return 0;
} }
release_console_sem();
orig_buf = buf; orig_buf = buf;
orig_count = count; orig_count = count;
if (from_user) { if (from_user) {
down(&con_buf_sem); down(&con_buf_sem);
again: again:
...@@ -1929,6 +1942,13 @@ static int do_con_write(struct tty_struct * tty, int from_user, ...@@ -1929,6 +1942,13 @@ static int do_con_write(struct tty_struct * tty, int from_user,
acquire_console_sem(); acquire_console_sem();
vt = (struct vt_struct *)tty->driver_data;
if (vt == NULL) {
printk(KERN_ERR "vt: argh, driver_data _became_ NULL !\n");
release_console_sem();
goto out;
}
himask = hi_font_mask; himask = hi_font_mask;
charmask = himask ? 0x1ff : 0xff; charmask = himask ? 0x1ff : 0xff;
...@@ -2442,14 +2462,16 @@ static int con_open(struct tty_struct *tty, struct file * filp) ...@@ -2442,14 +2462,16 @@ static int con_open(struct tty_struct *tty, struct file * filp)
acquire_console_sem(); acquire_console_sem();
i = vc_allocate(currcons); i = vc_allocate(currcons);
release_console_sem(); if (i) {
if (i) release_console_sem();
return i; return i;
}
vt_cons[currcons]->vc_num = currcons; vt_cons[currcons]->vc_num = currcons;
tty->driver_data = vt_cons[currcons]; tty->driver_data = vt_cons[currcons];
vc_cons[currcons].d->vc_tty = tty; vc_cons[currcons].d->vc_tty = tty;
release_console_sem();
if (!tty->winsize.ws_row && !tty->winsize.ws_col) { if (!tty->winsize.ws_row && !tty->winsize.ws_col) {
tty->winsize.ws_row = video_num_lines; tty->winsize.ws_row = video_num_lines;
tty->winsize.ws_col = video_num_columns; tty->winsize.ws_col = video_num_columns;
...@@ -2467,10 +2489,12 @@ static void con_close(struct tty_struct *tty, struct file * filp) ...@@ -2467,10 +2489,12 @@ static void con_close(struct tty_struct *tty, struct file * filp)
return; return;
vcs_remove_devfs(tty); vcs_remove_devfs(tty);
acquire_console_sem();
vt = (struct vt_struct*)tty->driver_data; vt = (struct vt_struct*)tty->driver_data;
if (vt) if (vt)
vc_cons[vt->vc_num].d->vc_tty = NULL; vc_cons[vt->vc_num].d->vc_tty = NULL;
tty->driver_data = 0; tty->driver_data = 0;
release_console_sem();
} }
static void vc_init(unsigned int currcons, unsigned int rows, unsigned int cols, int do_clear) static void vc_init(unsigned int currcons, unsigned int rows, unsigned int cols, int do_clear)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment