[PATCH] drivers/char/vt possible race

[Benjamin Herrenschmidt]

Hi !

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.

Andrew: I suggest putting that in -mm for a while, and if it
doesn't trigger any new problem, upstream, maybe without my
2 printk's "argh" :)

Ben.

diff -urN linux-2.5/drivers/char/vt.c linuxppc-2.5-benh/drivers/char/vt.c
--- linux-2.5/drivers/char/vt.c	2004-02-05 13:35:02.000000000 +1100
+++ linuxppc-2.5-benh/drivers/char/vt.c	2004-02-10 13:36:56.000000000 +1100
@@ -1883,14 +1883,24 @@
 	int c, tc, ok, n = 0, draw_x = -1;
 	unsigned int currcons;
 	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;
 	const unsigned char *orig_buf = NULL;
 	int orig_count;
 
 	if (in_interrupt())
 		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;
 	if (!vc_cons_allocated(currcons)) {
 	    /* could this happen? */
@@ -1899,13 +1909,16 @@
 		error = 1;
 		printk("con_write: tty %d not allocated\n", currcons+1);
 	    }
+	    release_console_sem();
 	    return 0;
 	}
+	release_console_sem();
 
 	orig_buf = buf;
 	orig_count = count;
 
 	if (from_user) {
+
 		down(&con_buf_sem);
 
 again:
@@ -1929,6 +1942,13 @@
 
 	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;
 	charmask = himask ? 0x1ff : 0xff;
 
@@ -2442,14 +2462,16 @@
 
 	acquire_console_sem();
 	i = vc_allocate(currcons);
-	release_console_sem();
-	if (i)
+	if (i) {
+		release_console_sem();
 		return i;
-
+	}
 	vt_cons[currcons]->vc_num = currcons;
 	tty->driver_data = vt_cons[currcons];
 	vc_cons[currcons].d->vc_tty = tty;
 
+	release_console_sem();
+
 	if (!tty->winsize.ws_row && !tty->winsize.ws_col) {
 		tty->winsize.ws_row = video_num_lines;
 		tty->winsize.ws_col = video_num_columns;
@@ -2467,10 +2489,12 @@
 		return;
 
 	vcs_remove_devfs(tty);
+	acquire_console_sem();
 	vt = (struct vt_struct*)tty->driver_data;
 	if (vt)
 		vc_cons[vt->vc_num].d->vc_tty = NULL;
 	tty->driver_data = 0;
+	release_console_sem();
 }
 
 static void vc_init(unsigned int currcons, unsigned int rows, unsigned int cols, int do_clear)


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/