Skip to contentSkip to conversation

make tty->count atomic_t

PATCH In review Jes Sorensen 2 participants 5 messages Apr 3, 2003 – Apr 4, 2003

Subsystems (12)
Networking Network drivers Arm i.MX and Layerscape MIPS +8 more subsystems
lore
Share & tools
lore

Jes Sorensen

author
#0 lore raw

PATCH 1/1 make tty->count atomic_t

Hi,

I believe the 2.4 tty code is racey in the way it handles tty->count. release_dev() does the tty->count-- thing without protecting against parallel execution, hence tty->count can end up a random state as tty->count-- isn't guaranteed to be atomic (load-store architectures and architectures with weak memory ordering etc).

The easy way to solve the problem is to change tty->count to an atomic_t.

Patch against 2.4.21-pre5-ac3, but also applies cleanly to 2.4.21-pre6. I have tried to fix places referencing the counter, my apologies if I missed any.

Comments, thoughts?

Cheers,
Jes

arch/alpha/kernel/srmcons.c +1 -1
diff -urN -X /home/jes/exclude-linux linux-2.4.21-pre5-ac3-vanilla/arch/alpha/kernel/srmcons.c linux-2.4.21-pre5-ac3/arch/alpha/kernel/srmcons.c
@@ -260,7 +260,7 @@
 
 	spin_lock_irqsave(&srmconsp->lock, flags);
 
-	if (tty->count == 1) {
+	if (atomic_read(&tty->count) == 1) {
 		srmconsp->tty = NULL;
 		del_timer(&srmconsp->timer);
 	}
arch/cris/drivers/serial.c +1 -1
diff -urN -X /home/jes/exclude-linux linux-2.4.21-pre5-ac3-vanilla/arch/cris/drivers/serial.c linux-2.4.21-pre5-ac3/arch/cris/drivers/serial.c
@@ -3293,7 +3293,7 @@
 	printk("[%d] rs_close ttyS%d, count = %d\n", current->pid, 
 	       info->line, info->count);
 #endif
-	if ((tty->count == 1) && (info->count != 1)) {
+	if ((atomic_read(&tty->count) == 1) && (info->count != 1)) {
 		/*
 		 * Uh, oh.  tty->count is 1, which means that the tty
 		 * structure will be freed.  Info->count should always
arch/mips/au1000/common/serial.c +2 -2
diff -urN -X /home/jes/exclude-linux linux-2.4.21-pre5-ac3-vanilla/arch/mips/au1000/common/serial.c linux-2.4.21-pre5-ac3/arch/mips/au1000/common/serial.c
@@ -195,7 +195,7 @@
 
 #if defined(MODULE) && defined(SERIAL_DEBUG_MCOUNT)
 #define DBG_CNT(s) printk("(%s): [%x] refc=%d, serc=%d, ttyc=%d -> %s\n", \
- kdevname(tty->device), (info->flags), serial_refcount,info->count,tty->count,s)
+ kdevname(tty->device), (info->flags), serial_refcount,info->count,atomic_read(&tty->count),s)
 #else
 #define DBG_CNT(s)
 #endif

@@ -1909,7 +1909,7 @@
 #ifdef SERIAL_DEBUG_OPEN
 	printk("rs_close ttys%d, count = %d\n", info->line, state->count);
 #endif
-	if ((tty->count == 1) && (state->count != 1)) {
+	if ((atomic_read(&tty->count) == 1) && (state->count != 1)) {
 		/*
 		 * Uh, oh.  tty->count is 1, which means that the tty
 		 * structure will be freed.  state->count should always
arch/mips/baget/vacserial.c +2 -2
diff -urN -X /home/jes/exclude-linux linux-2.4.21-pre5-ac3-vanilla/arch/mips/baget/vacserial.c linux-2.4.21-pre5-ac3/arch/mips/baget/vacserial.c
@@ -29,7 +29,7 @@
   
 #if defined(MODULE) && defined(SERIAL_DEBUG_MCOUNT)
 #define DBG_CNT(s) baget_printk("(%s):[%x] refc=%d, serc=%d, ttyc=%d-> %s\n", \
-  kdevname(tty->device),(info->flags),serial_refcount,info->count,tty->count,s)
+  kdevname(tty->device),(info->flags),serial_refcount,info->count,atomic_read(&tty->count),s)
 #else
 #define DBG_CNT(s)
 #endif

@@ -1658,7 +1658,7 @@
 	baget_printk("rs_close ttys%d, count = %d\n", 
 		     info->line, state->count);
 #endif
-	if ((tty->count == 1) && (state->count != 1)) {
+	if ((atomic_read(&tty->count) == 1) && (state->count != 1)) {
 		/*
 		 * Uh, oh.  tty->count is 1, which means that the tty
 		 * structure will be freed.  state->count should always
arch/ppc/8xx_io/uart.c +1 -1
diff -urN -X /home/jes/exclude-linux linux-2.4.21-pre5-ac3-vanilla/arch/ppc/8xx_io/uart.c linux-2.4.21-pre5-ac3/arch/ppc/8xx_io/uart.c
@@ -1678,7 +1678,7 @@
 #ifdef SERIAL_DEBUG_OPEN
 	printk("rs_close ttys%d, count = %d\n", info->line, state->count);
 #endif
-	if ((tty->count == 1) && (state->count != 1)) {
+	if ((atomic_read(&tty->count) == 1) && (state->count != 1)) {
 		/*
 		 * Uh, oh.  tty->count is 1, which means that the tty
 		 * structure will be freed.  state->count should always
drivers/char/amiserial.c +1 -1
diff -urN -X /home/jes/exclude-linux linux-2.4.21-pre5-ac3-vanilla/drivers/char/amiserial.c linux-2.4.21-pre5-ac3/drivers/char/amiserial.c
@@ -1544,7 +1544,7 @@
 #ifdef SERIAL_DEBUG_OPEN
 	printk("rs_close ttys%d, count = %d\n", info->line, state->count);
 #endif
-	if ((tty->count == 1) && (state->count != 1)) {
+	if ((atomic_read(&tty->count) == 1) && (state->count != 1)) {
 		/*
 		 * Uh, oh.  tty->count is 1, which means that the tty
 		 * structure will be freed.  state->count should always
drivers/char/console.c +2 -2
diff -urN -X /home/jes/exclude-linux linux-2.4.21-pre5-ac3-vanilla/drivers/char/console.c linux-2.4.21-pre5-ac3/drivers/char/console.c
@@ -2393,7 +2393,7 @@
 		tty->winsize.ws_row = video_num_lines;
 		tty->winsize.ws_col = video_num_columns;
 	}
-	if (tty->count == 1)
+	if (atomic_read(&tty->count) == 1)
 		vcs_make_devfs (currcons, 0);
 	return 0;
 }

@@ -2402,7 +2402,7 @@
 {
 	if (!tty)
 		return;
-	if (tty->count != 1) return;
+	if (atomic_read(&tty->count) != 1) return;
 	vcs_make_devfs (MINOR (tty->device) - tty->driver.minor_start, 1);
 	tty->driver_data = 0;
 }
drivers/char/cyclades.c +1 -1
diff -urN -X /home/jes/exclude-linux linux-2.4.21-pre5-ac3-vanilla/drivers/char/cyclades.c linux-2.4.21-pre5-ac3/drivers/char/cyclades.c
@@ -2822,7 +2822,7 @@
 #ifdef CY_DEBUG_OPEN
     printk("cyc:cy_close ttyC%d, count = %d\n", info->line, info->count);
 #endif
-    if ((tty->count == 1) && (info->count != 1)) {
+    if ((atomic_read(&tty->count) == 1) && (info->count != 1)) {
         /*
          * Uh, oh.  tty->count is 1, which means that the tty
          * structure will be freed.  Info->count should always
drivers/char/dz.c +1 -1
diff -urN -X /home/jes/exclude-linux linux-2.4.21-pre5-ac3-vanilla/drivers/char/dz.c linux-2.4.21-pre5-ac3/drivers/char/dz.c
@@ -1054,7 +1054,7 @@
 		restore_flags(flags);
 		return;
 	}
-	if ((tty->count == 1) && (info->count != 1)) {
+	if ((atomic_read(&tty->count) == 1) && (info->count != 1)) {
 		/*
 		 * Uh, oh.  tty->count is 1, which means that the tty
 		 * structure will be freed.  Info->count should always
drivers/char/esp.c +2 -2
diff -urN -X /home/jes/exclude-linux linux-2.4.21-pre5-ac3-vanilla/drivers/char/esp.c linux-2.4.21-pre5-ac3/drivers/char/esp.c
@@ -136,7 +136,7 @@
   
 #if defined(MODULE) && defined(SERIAL_DEBUG_MCOUNT)
 #define DBG_CNT(s) printk("(%s): [%x] refc=%d, serc=%d, ttyc=%d -> %s\n", \
- kdevname(tty->device), (info->flags), serial_refcount,info->count,tty->count,s)
+ kdevname(tty->device), (info->flags), serial_refcount,info->count,atomic_read(&tty->count),s)
 #else
 #define DBG_CNT(s)
 #endif

@@ -2051,7 +2051,7 @@
 #ifdef SERIAL_DEBUG_OPEN
 	printk("rs_close ttys%d, count = %d\n", info->line, info->count);
 #endif
-	if ((tty->count == 1) && (info->count != 1)) {
+	if ((atomic_read(&tty->count) == 1) && (info->count != 1)) {
 		/*
 		 * Uh, oh.  tty->count is 1, which means that the tty
 		 * structure will be freed.  Info->count should always
drivers/char/generic_serial.c +1 -1
diff -urN -X /home/jes/exclude-linux linux-2.4.21-pre5-ac3-vanilla/drivers/char/generic_serial.c linux-2.4.21-pre5-ac3/drivers/char/generic_serial.c
@@ -753,7 +753,7 @@
 		return;
 	}
 
-	if ((tty->count == 1) && (port->count != 1)) {
+	if ((atomic_read(&tty->count) == 1) && (port->count != 1)) {
 		printk(KERN_ERR "gs: gs_close: bad port count;"
 		       " tty->count is 1, port count is %d\n", port->count);
 		port->count = 1;
drivers/char/ip2main.c +4 -4
diff -urN -X /home/jes/exclude-linux linux-2.4.21-pre5-ac3-vanilla/drivers/char/ip2main.c linux-2.4.21-pre5-ac3/drivers/char/ip2main.c
@@ -372,7 +372,7 @@
 #if defined(MODULE) && defined(IP2DEBUG_OPEN)
 #define DBG_CNT(s) printk(KERN_DEBUG "(%s): [%x] refc=%d, ttyc=%d, modc=%x -> %s\n", \
 		    kdevname(tty->device),(pCh->flags),ref_count, \
-		    tty->count,/*GET_USE_COUNT(module)*/0,s)
+		    atomic_read(&tty->count),/*GET_USE_COUNT(module)*/0,s)
 #else
 #define DBG_CNT(s)
 #endif

@@ -1740,7 +1740,7 @@
 noblock:
 
 	/* first open - Assign termios structure to port */
-	if ( tty->count == 1 ) {
+	if ( atomic_read(&tty->count) == 1 ) {
 		i2QueueCommands(PTYPE_INLINE, pCh, 0, 2, CMD_CTSFL_DSAB, CMD_RTSFL_DSAB);
 		if ( pCh->flags & ASYNC_SPLIT_TERMIOS ) {
 			if ( tty->driver.subtype == SERIAL_TYPE_NORMAL ) {

@@ -1805,7 +1805,7 @@
 
 		return;
 	}
-	if ( tty->count > 1 ) { /* not the last close */
+	if ( atomic_read(&tty->count) > 1 ) { /* not the last close */
 		MOD_DEC_USE_COUNT;
 
 		ip2trace (CHANN, ITRC_CLOSE, 2, 1, 3 );

@@ -3364,7 +3364,7 @@
 		pCh = DevTable[i];
 		if (pCh) {
 			tty = pCh->pTTY;
-			if (tty && tty->count) {
+			if (tty && atomic_read(&tty->count)) {
 				len += sprintf(buf+len,FMTLINE,i,(int)tty->flags,pCh->flags,
 									tty->termios->c_cflag,tty->termios->c_iflag);
drivers/char/isicom.c +1 -1
diff -urN -X /home/jes/exclude-linux linux-2.4.21-pre5-ac3-vanilla/drivers/char/isicom.c linux-2.4.21-pre5-ac3/drivers/char/isicom.c
@@ -1160,7 +1160,7 @@
 		return;
 	}
 	
-	if ((tty->count == 1) && (port->count != 1)) {
+	if ((atomic_read(&tty->count) == 1) && (port->count != 1)) {
 		printk(KERN_WARNING "ISICOM:(0x%x) isicom_close: bad port count"
 			"tty->count = 1	port count = %d.\n",
 			card->base, port->count);
drivers/char/istallion.c +1 -1
diff -urN -X /home/jes/exclude-linux linux-2.4.21-pre5-ac3-vanilla/drivers/char/istallion.c linux-2.4.21-pre5-ac3/drivers/char/istallion.c
@@ -1173,7 +1173,7 @@
 		restore_flags(flags);
 		return;
 	}
-	if ((tty->count == 1) && (portp->refcount != 1))
+	if ((atomic_read(&tty->count) == 1) && (portp->refcount != 1))
 		portp->refcount = 1;
 	if (portp->refcount-- > 1) {
 		MOD_DEC_USE_COUNT;
drivers/char/moxa.c +1 -1
diff -urN -X /home/jes/exclude-linux linux-2.4.21-pre5-ac3-vanilla/drivers/char/moxa.c linux-2.4.21-pre5-ac3/drivers/char/moxa.c
@@ -642,7 +642,7 @@
 	}
 	ch = (struct moxa_str *) tty->driver_data;
 
-	if ((tty->count == 1) && (ch->count != 1)) {
+	if ((atomic_read(&tty->count) == 1) && (ch->count != 1)) {
 		printk("moxa_close: bad serial port count; tty->count is 1, "
 		       "ch->count is %d\n", ch->count);
 		ch->count = 1;
drivers/char/mxser.c +1 -1
diff -urN -X /home/jes/exclude-linux linux-2.4.21-pre5-ac3-vanilla/drivers/char/mxser.c linux-2.4.21-pre5-ac3/drivers/char/mxser.c
@@ -824,7 +824,7 @@
 		MOD_DEC_USE_COUNT;
 		return;
 	}
-	if ((tty->count == 1) && (info->count != 1)) {
+	if ((atomic_read(&tty->count) == 1) && (info->count != 1)) {
 		/*
 		 * Uh, oh.        tty->count is 1, which means that the tty
 		 * structure will be freed.  Info->count should always
drivers/char/n_tty.c +3 -2
diff -urN -X /home/jes/exclude-linux linux-2.4.21-pre5-ac3-vanilla/drivers/char/n_tty.c linux-2.4.21-pre5-ac3/drivers/char/n_tty.c
@@ -119,7 +119,7 @@
  */
 static void check_unthrottle(struct tty_struct * tty)
 {
-	if (tty->count &&
+	if (atomic_read(&tty->count) &&
 	    test_and_clear_bit(TTY_THROTTLED, &tty->flags) && 
 	    tty->driver.unthrottle)
 		tty->driver.unthrottle(tty);

@@ -1170,7 +1170,8 @@
 			retval = -ERESTARTSYS;
 			break;
 		}
-		if (tty_hung_up_p(file) || (tty->link && !tty->link->count)) {
+		if (tty_hung_up_p(file) ||
+		    (tty->link && !atomic_read(&tty->link->count))) {
 			retval = -EIO;
 			break;
 		}
drivers/char/pcxx.c +1 -1
diff -urN -X /home/jes/exclude-linux linux-2.4.21-pre5-ac3-vanilla/drivers/char/pcxx.c linux-2.4.21-pre5-ac3/drivers/char/pcxx.c
@@ -581,7 +581,7 @@
 			return;
 		}
 		/* this check is in serial.c, it won't hurt to do it here too */
-		if ((tty->count == 1) && (info->count != 1)) {
+		if ((atomic_read(&tty->count) == 1) && (info->count != 1)) {
 			/*
 			 * Uh, oh.  tty->count is 1, which means that the tty
 			 * structure will be freed.  Info->count should always
drivers/char/pty.c +9 -4
diff -urN -X /home/jes/exclude-linux linux-2.4.21-pre5-ac3-vanilla/drivers/char/pty.c linux-2.4.21-pre5-ac3/drivers/char/pty.c
@@ -70,13 +70,18 @@
 
 static void pty_close(struct tty_struct * tty, struct file * filp)
 {
+	int count;
+
 	if (!tty)
 		return;
+
+	count = atomic_read(&tty->count);
 	if (tty->driver.subtype == PTY_TYPE_MASTER) {
-		if (tty->count > 1)
-			printk("master pty_close: count = %d!!\n", tty->count);
+		if (count > 1)
+			printk("master pty_close: count = %d!!\n",
+			       atomic_read(&tty->count));
 	} else {
-		if (tty->count > 2)
+		if (count > 2)
 			return;
 	}
 	wake_up_interruptible(&tty->read_wait);

@@ -329,7 +334,7 @@
 		goto out;
 	if (test_bit(TTY_PTY_LOCK, &tty->link->flags))
 		goto out;
-	if (tty->link->count != 1)
+	if (atomic_read(&tty->link->count) != 1)
 		goto out;
 
 	clear_bit(TTY_OTHER_CLOSED, &tty->link->flags);
drivers/char/riscom8.c +1 -1
diff -urN -X /home/jes/exclude-linux linux-2.4.21-pre5-ac3-vanilla/drivers/char/riscom8.c linux-2.4.21-pre5-ac3/drivers/char/riscom8.c
@@ -1142,7 +1142,7 @@
 		goto out;
 	
 	bp = port_Board(port);
-	if ((tty->count == 1) && (port->count != 1))  {
+	if ((atomic_read(&tty->count) == 1) && (port->count != 1))  {
 		printk(KERN_INFO "rc%d: rc_close: bad port count;"
 		       " tty->count is 1, port count is %d\n",
 		       board_No(bp), port->count);
drivers/char/rocket.c +1 -1
diff -urN -X /home/jes/exclude-linux linux-2.4.21-pre5-ac3-vanilla/drivers/char/rocket.c linux-2.4.21-pre5-ac3/drivers/char/rocket.c
@@ -1052,7 +1052,7 @@
 		restore_flags(flags);
 		return;
 	}
-	if ((tty->count == 1) && (info->count != 1)) {
+	if ((atomic_read(&tty->count) == 1) && (info->count != 1)) {
 		/*
 		 * Uh, oh.  tty->count is 1, which means that the tty
 		 * structure will be freed.  Info->count should always
drivers/char/serial.c +2 -2
diff -urN -X /home/jes/exclude-linux linux-2.4.21-pre5-ac3-vanilla/drivers/char/serial.c linux-2.4.21-pre5-ac3/drivers/char/serial.c
@@ -375,7 +375,7 @@
 
 #if defined(MODULE) && defined(SERIAL_DEBUG_MCOUNT)
 #define DBG_CNT(s) printk("(%s): [%x] refc=%d, serc=%d, ttyc=%d -> %s\n", \
- kdevname(tty->device), (info->flags), serial_refcount,info->count,tty->count,s)
+ kdevname(tty->device), (info->flags), serial_refcount,info->count,atomic_read(&tty->count),s)
 #else
 #define DBG_CNT(s)
 #endif

@@ -2797,7 +2797,7 @@
 #ifdef SERIAL_DEBUG_OPEN
 	printk("rs_close ttys%d, count = %d\n", info->line, state->count);
 #endif
-	if ((tty->count == 1) && (state->count != 1)) {
+	if ((atomic_read(&tty->count) == 1) && (state->count != 1)) {
 		/*
 		 * Uh, oh.  tty->count is 1, which means that the tty
 		 * structure will be freed.  state->count should always
drivers/char/serial167.c +1 -1
diff -urN -X /home/jes/exclude-linux linux-2.4.21-pre5-ac3-vanilla/drivers/char/serial167.c linux-2.4.21-pre5-ac3/drivers/char/serial167.c
@@ -1877,7 +1877,7 @@
     printk("cy_close ttyS%d, count = %d\n", info->line, info->count);
 #endif
 
-    if ((tty->count == 1) && (info->count != 1)) {
+    if ((atomic_read(&tty->count) == 1) && (info->count != 1)) {
 	/*
 	 * Uh, oh.  tty->count is 1, which means that the tty
 	 * structure will be freed.  Info->count should always
drivers/char/serial_amba.c +1 -1
diff -urN -X /home/jes/exclude-linux linux-2.4.21-pre5-ac3-vanilla/drivers/char/serial_amba.c linux-2.4.21-pre5-ac3/drivers/char/serial_amba.c
@@ -1404,7 +1404,7 @@
 		return;
 	}
 
-	if ((tty->count == 1) && (state->count != 1)) {
+	if ((atomic_read(&tty->count) == 1) && (state->count != 1)) {
 		/*
 		 * Uh, oh.  tty->count is 1, which means that the tty
 		 * structure will be freed.  state->count should always
drivers/char/serial_txx927.c +2 -2
diff -urN -X /home/jes/exclude-linux linux-2.4.21-pre5-ac3-vanilla/drivers/char/serial_txx927.c linux-2.4.21-pre5-ac3/drivers/char/serial_txx927.c
@@ -155,7 +155,7 @@
 
 #if defined(MODULE) && defined(SERIAL_DEBUG_MCOUNT)
 #define DBG_CNT(s) printk("(%s): [%x] refc=%d, serc=%d, ttyc=%d -> %s\n", \
- kdevname(tty->device), (info->flags), serial_refcount,info->count,tty->count,s)
+ kdevname(tty->device), (info->flags), serial_refcount,info->count,atomic_read(&tty->count),s)
 #else
 #define DBG_CNT(s)
 #endif                                     

@@ -1407,7 +1407,7 @@
 #ifdef SERIAL_DEBUG_OPEN
 	printk("rs_close ttys%d, count = %d\n", info->line, state->count);
 #endif
-	if ((tty->count == 1) && (state->count != 1)) {
+	if ((atomic_read(&tty->count) == 1) && (state->count != 1)) {
 		/*
 		 * Uh, oh.  tty->count is 1, which means that the tty
 		 * structure will be freed.  state->count should always
drivers/char/specialix.c +1 -1
diff -urN -X /home/jes/exclude-linux linux-2.4.21-pre5-ac3-vanilla/drivers/char/specialix.c linux-2.4.21-pre5-ac3/drivers/char/specialix.c
@@ -1517,7 +1517,7 @@
 	}
 	
 	bp = port_Board(port);
-	if ((tty->count == 1) && (port->count != 1)) {
+	if ((atomic_read(&tty->count) == 1) && (port->count != 1)) {
 		printk(KERN_ERR "sx%d: sx_close: bad port count;"
 		       " tty->count is 1, port count is %d\n",
 		       board_No(bp), port->count);
drivers/char/stallion.c +1 -1
diff -urN -X /home/jes/exclude-linux linux-2.4.21-pre5-ac3-vanilla/drivers/char/stallion.c linux-2.4.21-pre5-ac3/drivers/char/stallion.c
@@ -1197,7 +1197,7 @@
 		restore_flags(flags);
 		return;
 	}
-	if ((tty->count == 1) && (portp->refcount != 1))
+	if ((atomic_read(&tty->count) == 1) && (portp->refcount != 1))
 		portp->refcount = 1;
 	if (portp->refcount-- > 1) {
 		MOD_DEC_USE_COUNT;
drivers/char/synclink.c +1 -1
diff -urN -X /home/jes/exclude-linux linux-2.4.21-pre5-ac3-vanilla/drivers/char/synclink.c linux-2.4.21-pre5-ac3/drivers/char/synclink.c
@@ -3261,7 +3261,7 @@
 	if (!info->count || tty_hung_up_p(filp))
 		goto cleanup;
 			
-	if ((tty->count == 1) && (info->count != 1)) {
+	if ((atomic_read(&tty->count) == 1) && (info->count != 1)) {
 		/*
 		 * tty->count is 1 and the tty structure will be freed.
 		 * info->count should be one in this case.
drivers/char/synclinkmp.c +1 -1
diff -urN -X /home/jes/exclude-linux linux-2.4.21-pre5-ac3-vanilla/drivers/char/synclinkmp.c linux-2.4.21-pre5-ac3/drivers/char/synclinkmp.c
@@ -851,7 +851,7 @@
 	if (!info->count || tty_hung_up_p(filp))
 		goto cleanup;
 
-	if ((tty->count == 1) && (info->count != 1)) {
+	if ((atomic_read(&tty->count) == 1) && (info->count != 1)) {
 		/*
 		 * tty->count is 1 and the tty structure will be freed.
 		 * info->count should be one in this case.
drivers/char/tty_io.c +23 -19

104 lines not on this page · raw message

drivers/char/vt.c +2 -1
diff -urN -X /home/jes/exclude-linux linux-2.4.21-pre5-ac3-vanilla/drivers/char/vt.c linux-2.4.21-pre5-ac3/drivers/char/vt.c
@@ -40,7 +40,8 @@
 char vt_dont_switch;
 extern struct tty_driver console_driver;
 
-#define VT_IS_IN_USE(i)	(console_driver.table[i] && console_driver.table[i]->count)
+#define VT_IS_IN_USE(i)	(console_driver.table[i] && \
+			 atomic_read(&console_driver.table[i]->count))
 #define VT_BUSY(i)	(VT_IS_IN_USE(i) || i == fg_console || i == sel_cons)
 
 /*
drivers/isdn/isdn_tty.c +1 -1
diff -urN -X /home/jes/exclude-linux linux-2.4.21-pre5-ac3-vanilla/drivers/isdn/isdn_tty.c linux-2.4.21-pre5-ac3/drivers/isdn/isdn_tty.c
@@ -1807,7 +1807,7 @@
 #endif
 		return;
 	}
-	if ((tty->count == 1) && (info->count != 1)) {
+	if ((atomic_read(&tty->count) == 1) && (info->count != 1)) {
 		/*
 		 * Uh, oh.  tty->count is 1, which means that the tty
 		 * structure will be freed.  Info->count should always
drivers/macintosh/macserial.c +1 -1
diff -urN -X /home/jes/exclude-linux linux-2.4.21-pre5-ac3-vanilla/drivers/macintosh/macserial.c linux-2.4.21-pre5-ac3/drivers/macintosh/macserial.c
@@ -1957,7 +1957,7 @@
 	}
 
 	OPNDBG("rs_close ttyS%d, count = %d\n", info->line, info->count);
-	if ((tty->count == 1) && (info->count != 1)) {
+	if ((atomic_read(&tty->count) == 1) && (info->count != 1)) {
 		/*
 		 * Uh, oh.  tty->count is 1, which means that the tty
 		 * structure will be freed.  Info->count should always
drivers/net/wan/8253x/8253xsyn.c +1 -1
diff -urN -X /home/jes/exclude-linux linux-2.4.21-pre5-ac3-vanilla/drivers/net/wan/8253x/8253xsyn.c linux-2.4.21-pre5-ac3/drivers/net/wan/8253x/8253xsyn.c
@@ -1030,7 +1030,7 @@
 	}
 	
 #if 0
-	if ((tty->count == 1) && (port->count != 0)) 
+	if ((atomic_read(&tty->count) == 1) && (port->count != 0)) 
 	{
 		/*
 		 * Uh, oh.  tty->count is 1, which means that the tty
drivers/s390/char/con3215.c +1 -1
diff -urN -X /home/jes/exclude-linux linux-2.4.21-pre5-ac3-vanilla/drivers/s390/char/con3215.c linux-2.4.21-pre5-ac3/drivers/s390/char/con3215.c
@@ -897,7 +897,7 @@
 	raw3215_info *raw;
 
 	raw = (raw3215_info *) tty->driver_data;
-	if (raw == NULL || tty->count > 1)
+	if (raw == NULL || atomic_read(&tty->count) > 1)
 		return;
 	tty->closing = 1;
 	/* Shutdown the terminal */
drivers/s390/char/hwc_tty.c +1 -1
diff -urN -X /home/jes/exclude-linux linux-2.4.21-pre5-ac3-vanilla/drivers/s390/char/hwc_tty.c linux-2.4.21-pre5-ac3/drivers/s390/char/hwc_tty.c
@@ -83,7 +83,7 @@
 			"do not close hwc tty because of wrong device number");
 		return;
 	}
-	if (tty->count > 1)
+	if (atomic_read(&tty->count) > 1)
 		return;
 
 	hwc_tty_data.tty = NULL;
drivers/s390/net/ctctty.c +1 -1
diff -urN -X /home/jes/exclude-linux linux-2.4.21-pre5-ac3-vanilla/drivers/s390/net/ctctty.c linux-2.4.21-pre5-ac3/drivers/s390/net/ctctty.c
@@ -1066,7 +1066,7 @@
 #endif
 		return;
 	}
-	if ((tty->count == 1) && (info->count != 1)) {
+	if ((atomic_read(&tty->count) == 1) && (info->count != 1)) {
 		/*
 		 * Uh, oh.  tty->count is 1, which means that the tty
 		 * structure will be freed.  Info->count should always
drivers/sbus/char/aurora.c +1 -1
diff -urN -X /home/jes/exclude-linux linux-2.4.21-pre5-ac3-vanilla/drivers/sbus/char/aurora.c linux-2.4.21-pre5-ac3/drivers/sbus/char/aurora.c
@@ -1504,7 +1504,7 @@
 	}
 	
 	bp = port_Board(port);
-	if ((tty->count == 1) && (port->count != 1))  {
+	if ((atomic_read(&tty->count) == 1) && (port->count != 1))  {
 		printk(KERN_DEBUG "aurora%d: aurora_close: bad port count; "
 		       "tty->count is 1, port count is %d\n",
 		       board_No(bp), port->count);
drivers/sbus/char/sab82532.c +1 -1
diff -urN -X /home/jes/exclude-linux linux-2.4.21-pre5-ac3-vanilla/drivers/sbus/char/sab82532.c linux-2.4.21-pre5-ac3/drivers/sbus/char/sab82532.c
@@ -1610,7 +1610,7 @@
 #ifdef SERIAL_DEBUG_OPEN
 	printk("sab82532_close ttys%d, count = %d\n", info->line, info->count);
 #endif
-	if ((tty->count == 1) && (info->count != 1)) {
+	if ((atomic_read(&tty->count) == 1) && (info->count != 1)) {
 		/*
 		 * Uh, oh.  tty->count is 1, which means that the tty
 		 * structure will be freed.  info->count should always
drivers/sbus/char/su.c +2 -2
diff -urN -X /home/jes/exclude-linux linux-2.4.21-pre5-ac3-vanilla/drivers/sbus/char/su.c linux-2.4.21-pre5-ac3/drivers/sbus/char/su.c
@@ -39,7 +39,7 @@
 do {									\
 	printk("(%s): [%x] refc=%d, serc=%d, ttyc=%d -> %s\n",		\
 	       kdevname(tty->device), (info->flags), serial_refcount,	\
-	       info->count,tty->count,s);				\
+	       info->count,atomic_read(&tty->count),s);			\
 } while (0)
 #else
 #define DBG_CNT(s)

@@ -1756,7 +1756,7 @@
 #ifdef SERIAL_DEBUG_OPEN
 	printk("su_close ttys%d, count = %d\n", info->line, info->count);
 #endif
-	if ((tty->count == 1) && (info->count != 1)) {
+	if ((atomic_read(&tty->count) == 1) && (info->count != 1)) {
 		/*
 		 * Uh, oh.  tty->count is 1, which means that the tty
 		 * structure will be freed.  info->count should always
drivers/sbus/char/zs.c +1 -1
diff -urN -X /home/jes/exclude-linux linux-2.4.21-pre5-ac3-vanilla/drivers/sbus/char/zs.c linux-2.4.21-pre5-ac3/drivers/sbus/char/zs.c
@@ -1547,7 +1547,7 @@
 #ifdef SERIAL_DEBUG_OPEN
 	printk("zs_close tty-%d, count = %d\n", info->line, info->count);
 #endif
-	if ((tty->count == 1) && (info->count != 1)) {
+	if ((atomic_read(&tty->count) == 1) && (info->count != 1)) {
 		/*
 		 * Uh, oh.  tty->count is 1, which means that the tty
 		 * structure will be freed.  Info->count should always
drivers/sgi/char/sgiserial.c +1 -1
diff -urN -X /home/jes/exclude-linux linux-2.4.21-pre5-ac3-vanilla/drivers/sgi/char/sgiserial.c linux-2.4.21-pre5-ac3/drivers/sgi/char/sgiserial.c
@@ -1489,7 +1489,7 @@
 #ifdef SERIAL_DEBUG_OPEN
 	printk("rs_close ttys%d, count = %d\n", info->line, info->count);
 #endif
-	if ((tty->count == 1) && (info->count != 1)) {
+	if ((atomic_read(&tty->count) == 1) && (info->count != 1)) {
 		/*
 		 * Uh, oh.  tty->count is 1, which means that the tty
 		 * structure will be freed.  Info->count should always
drivers/tc/zs.c +1 -1
diff -urN -X /home/jes/exclude-linux linux-2.4.21-pre5-ac3-vanilla/drivers/tc/zs.c linux-2.4.21-pre5-ac3/drivers/tc/zs.c
@@ -1374,7 +1374,7 @@
 #ifdef SERIAL_DEBUG_OPEN
 	printk("rs_close ttyS%02d, count = %d\n", info->line, info->count);
 #endif
-	if ((tty->count == 1) && (info->count != 1)) {
+	if ((atomic_read(&tty->count) == 1) && (info->count != 1)) {
 		/*
 		 * Uh, oh.  tty->count is 1, which means that the tty
 		 * structure will be freed.  Info->count should always
include/linux/tty.h +1 -1
diff -urN -X /home/jes/exclude-linux linux-2.4.21-pre5-ac3-vanilla/include/linux/tty.h linux-2.4.21-pre5-ac3/include/linux/tty.h
@@ -265,7 +265,7 @@
 	int session;
 	kdev_t	device;
 	unsigned long flags;
-	int count;
+	atomic_t count;
 	struct winsize winsize;
 	unsigned char stopped:1, hw_stopped:1, flow_stopped:1, packet:1;
 	unsigned char low_latency:1, warned:1;
net/irda/ircomm/ircomm_tty.c +1 -1
diff -urN -X /home/jes/exclude-linux linux-2.4.21-pre5-ac3-vanilla/net/irda/ircomm/ircomm_tty.c linux-2.4.21-pre5-ac3/net/irda/ircomm/ircomm_tty.c
@@ -526,7 +526,7 @@
 		return;
 	}
 
-	if ((tty->count == 1) && (self->open_count != 1)) {
+	if ((atomic_read(&tty->count) == 1) && (self->open_count != 1)) {
 		/*
 		 * Uh, oh.  tty->count is 1, which means that the tty
 		 * structure will be freed.  state->count should always
#1 lore raw
Quoted 5 lines · Jes Sorensen

I believe the 2.4 tty code is racey in the way it handles tty->count. release_dev() does the tty->count-- thing without protecting against parallel execution, hence tty->count can end up a random state as tty->count-- isn't guaranteed to be atomic (load-store architectures and architectures with weak memory ordering etc).

Isn't release_dev() only called under the BKL, which guarantees the old "single-thread in the kernel at a time" behaviour from pre-SMP Linux ?

Signature · 2 lines
Russell King ([email protected])                The developer of ARM Linux
             http://www.arm.linux.org.uk/personal/aboutme.html
author
#2 lore raw
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

"Russell" == Russell King <[email protected]> writes:

Quoted 4 lines · Jes Sorensen
Quoted 4 lines

I believe the 2.4 tty code is racey in the way it handles tty->count. release_dev() does the tty->count-- thing without protecting against parallel execution, hence tty->count can end up a random state as

tty-> count-- isn't guaranteed to be atomic (load-store architectures tty-> and

Quoted 1 line
Quoted 1 line

architectures with weak memory ordering etc).

Russell> Isn't release_dev() only called under the BKL, which Russell> guarantees the old "single-thread in the kernel at a time" Russell> behaviour from pre-SMP Linux ?

It's called from tty_release() and tty_open(). tty_release() grabs the BKL but I don't see the path that grabs it when calling through tty_open() (doesn't mean I am not blind of course ;-).

cheers,
Jes

#3 lore raw
Quoted 10 lines · Jes Sorensen
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

"Russell" == Russell King <[email protected]> writes:

Quoted 4 lines · Jes Sorensen
Quoted 4 lines

I believe the 2.4 tty code is racey in the way it handles tty->count. release_dev() does the tty->count-- thing without protecting against parallel execution, hence tty->count can end up a random state as

tty-> count-- isn't guaranteed to be atomic (load-store architectures tty-> and

Quoted 1 line
Quoted 1 line

architectures with weak memory ordering etc).

Quoted 7 lines

Russell> Isn't release_dev() only called under the BKL, which Russell> guarantees the old "single-thread in the kernel at a time" Russell> behaviour from pre-SMP Linux ?

It's called from tty_release() and tty_open(). tty_release() grabs the BKL but I don't see the path that grabs it when calling through tty_open() (doesn't mean I am not blind of course ;-).

It's hidden away in fs/devices.c:chrdev_open()

Signature · 2 lines
Russell King ([email protected])                The developer of ARM Linux
             http://www.arm.linux.org.uk/personal/aboutme.html
author
#4 lore raw
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line

"Russell" == Russell King <[email protected]> writes:

Russell> On Thu, Apr 03, 2003 at 06:18:22PM -0500, Jes Sorensen wrote:
Russell> Isn't release_dev() only called under the BKL, which Russell> guarantees the old "single-thread in the kernel at a time" Russell> behaviour from pre-SMP Linux ?

Quoted 3 lines
Quoted 3 lines

It's called from tty_release() and tty_open(). tty_release() grabs
the BKL but I don't see the path that grabs it when calling through tty_open() (doesn't mean I am not blind of course ;-).

Russell> It's hidden away in fs/devices.c:chrdev_open()

Good point, ok back to the drawing broad ;-(

Thanks,
Jes