lists.openwall.net   lists  /  announce  owl-users  owl-dev  john-users  john-dev  passwdqc-users  yescrypt  popa3d-users  /  oss-security  kernel-hardening  musl  sabotage  tlsify  passwords  /  crypt-dev  xvendor  /  Bugtraq  Full-Disclosure  linux-kernel  linux-netdev  linux-ext4  linux-hardening  linux-cve-announce  PHC 
Open Source and information security mailing list archives
 
Hash Suite for Android: free password hash cracker in your pocket
[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-Id: <1260574122-10676-45-git-send-email-gregkh@suse.de>
Date:	Fri, 11 Dec 2009 15:28:29 -0800
From:	Greg Kroah-Hartman <gregkh@...e.de>
To:	linux-kernel@...r.kernel.org
Cc:	Alan Cox <alan@...ux.intel.com>,
	Greg Kroah-Hartman <gregkh@...e.de>
Subject: [PATCH 45/58] tty: mxser: Use the new locking rules to fix setserial properly

From: Alan Cox <alan@...ux.intel.com>

Propogate the init/shutdown mutex through the setserial logic. Use the proper
locks for the various bits still using the BKL. Kill the BKL in this driver.

Updated to fix the bug noted by Dan Carpenter

Signed-off-by: Alan Cox <alan@...ux.intel.com>
Signed-off-by: Greg Kroah-Hartman <gregkh@...e.de>
---
 drivers/char/mxser.c |  145 +++++++++++++++++++++++++++-----------------------
 1 files changed, 78 insertions(+), 67 deletions(-)

diff --git a/drivers/char/mxser.c b/drivers/char/mxser.c
index 9dac516..3d92306 100644
--- a/drivers/char/mxser.c
+++ b/drivers/char/mxser.c
@@ -23,7 +23,6 @@
 #include <linux/errno.h>
 #include <linux/signal.h>
 #include <linux/sched.h>
-#include <linux/smp_lock.h>
 #include <linux/timer.h>
 #include <linux/interrupt.h>
 #include <linux/tty.h>
@@ -1229,6 +1228,7 @@ static int mxser_set_serial_info(struct tty_struct *tty,
 		struct serial_struct __user *new_info)
 {
 	struct mxser_port *info = tty->driver_data;
+	struct tty_port *port = &info->port;
 	struct serial_struct new_serial;
 	speed_t baud;
 	unsigned long sl_flags;
@@ -1244,7 +1244,7 @@ static int mxser_set_serial_info(struct tty_struct *tty,
 			new_serial.port != info->ioaddr)
 		return -EINVAL;
 
-	flags = info->port.flags & ASYNC_SPD_MASK;
+	flags = port->flags & ASYNC_SPD_MASK;
 
 	if (!capable(CAP_SYS_ADMIN)) {
 		if ((new_serial.baud_base != info->baud_base) ||
@@ -1258,16 +1258,17 @@ static int mxser_set_serial_info(struct tty_struct *tty,
 		 * OK, past this point, all the error checking has been done.
 		 * At this point, we start making changes.....
 		 */
-		info->port.flags = ((info->port.flags & ~ASYNC_FLAGS) |
+		port->flags = ((port->flags & ~ASYNC_FLAGS) |
 				(new_serial.flags & ASYNC_FLAGS));
-		info->port.close_delay = new_serial.close_delay * HZ / 100;
-		info->port.closing_wait = new_serial.closing_wait * HZ / 100;
-		tty->low_latency = (info->port.flags & ASYNC_LOW_LATENCY)
-								? 1 : 0;
-		if ((info->port.flags & ASYNC_SPD_MASK) == ASYNC_SPD_CUST &&
+		port->close_delay = new_serial.close_delay * HZ / 100;
+		port->closing_wait = new_serial.closing_wait * HZ / 100;
+		tty->low_latency = (port->flags & ASYNC_LOW_LATENCY) ? 1 : 0;
+		if ((port->flags & ASYNC_SPD_MASK) == ASYNC_SPD_CUST &&
 				(new_serial.baud_base != info->baud_base ||
 				new_serial.custom_divisor !=
 				info->custom_divisor)) {
+			if (new_serial.custom_divisor == 0)
+				return -EINVAL;
 			baud = new_serial.baud_base / new_serial.custom_divisor;
 			tty_encode_baud_rate(tty, baud, baud);
 		}
@@ -1277,18 +1278,16 @@ static int mxser_set_serial_info(struct tty_struct *tty,
 
 	process_txrx_fifo(info);
 
-	if (info->port.flags & ASYNC_INITIALIZED) {
-		if (flags != (info->port.flags & ASYNC_SPD_MASK)) {
+	if (test_bit(ASYNCB_INITIALIZED, &port->flags)) {
+		if (flags != (port->flags & ASYNC_SPD_MASK)) {
 			spin_lock_irqsave(&info->slock, sl_flags);
 			mxser_change_speed(tty, NULL);
 			spin_unlock_irqrestore(&info->slock, sl_flags);
 		}
 	} else {
-		mutex_lock(&info->port.mutex);
-		retval = mxser_activate(&info->port, tty);
+		retval = mxser_activate(port, tty);
 		if (retval == 0)
-			set_bit(ASYNCB_INITIALIZED, &info->port.flags);
-		mutex_unlock(&info->port.mutex);
+			set_bit(ASYNCB_INITIALIZED, &port->flags);
 	}
 	return retval;
 }
@@ -1478,7 +1477,8 @@ static int __init mxser_read_register(int port, unsigned short *regs)
 
 static int mxser_ioctl_special(unsigned int cmd, void __user *argp)
 {
-	struct mxser_port *port;
+	struct mxser_port *ip;
+	struct tty_port *port;
 	struct tty_struct *tty;
 	int result, status;
 	unsigned int i, j;
@@ -1494,38 +1494,39 @@ static int mxser_ioctl_special(unsigned int cmd, void __user *argp)
 
 	case MOXA_CHKPORTENABLE:
 		result = 0;
-		lock_kernel();
 		for (i = 0; i < MXSER_BOARDS; i++)
 			for (j = 0; j < MXSER_PORTS_PER_BOARD; j++)
 				if (mxser_boards[i].ports[j].ioaddr)
 					result |= (1 << i);
-		unlock_kernel();
 		return put_user(result, (unsigned long __user *)argp);
 	case MOXA_GETDATACOUNT:
-		lock_kernel();
+		/* The receive side is locked by port->slock but it isn't
+		   clear that an exact snapshot is worth copying here */
 		if (copy_to_user(argp, &mxvar_log, sizeof(mxvar_log)))
 			ret = -EFAULT;
-		unlock_kernel();
 		return ret;
 	case MOXA_GETMSTATUS: {
 		struct mxser_mstatus ms, __user *msu = argp;
-		lock_kernel();
 		for (i = 0; i < MXSER_BOARDS; i++)
 			for (j = 0; j < MXSER_PORTS_PER_BOARD; j++) {
-				port = &mxser_boards[i].ports[j];
+				ip = &mxser_boards[i].ports[j];
+				port = &ip->port;
 				memset(&ms, 0, sizeof(ms));
 
-				if (!port->ioaddr)
+				mutex_lock(&port->mutex);
+				if (!ip->ioaddr)
 					goto copy;
 				
-				tty = tty_port_tty_get(&port->port);
+				tty = tty_port_tty_get(port);
 
 				if (!tty || !tty->termios)
-					ms.cflag = port->normal_termios.c_cflag;
+					ms.cflag = ip->normal_termios.c_cflag;
 				else
 					ms.cflag = tty->termios->c_cflag;
 				tty_kref_put(tty);
-				status = inb(port->ioaddr + UART_MSR);
+				spin_lock_irq(&ip->slock);
+				status = inb(ip->ioaddr + UART_MSR);
+				spin_unlock_irq(&ip->slock);
 				if (status & UART_MSR_DCD)
 					ms.dcd = 1;
 				if (status & UART_MSR_DSR)
@@ -1533,13 +1534,11 @@ static int mxser_ioctl_special(unsigned int cmd, void __user *argp)
 				if (status & UART_MSR_CTS)
 					ms.cts = 1;
 			copy:
-				if (copy_to_user(msu, &ms, sizeof(ms))) {
-					unlock_kernel();
+				mutex_unlock(&port->mutex);
+				if (copy_to_user(msu, &ms, sizeof(ms)))
 					return -EFAULT;
-				}
 				msu++;
 			}
-		unlock_kernel();
 		return 0;
 	}
 	case MOXA_ASPP_MON_EXT: {
@@ -1551,41 +1550,48 @@ static int mxser_ioctl_special(unsigned int cmd, void __user *argp)
 		if (!me)
 			return -ENOMEM;
 
-		lock_kernel();
 		for (i = 0, p = 0; i < MXSER_BOARDS; i++) {
 			for (j = 0; j < MXSER_PORTS_PER_BOARD; j++, p++) {
 				if (p >= ARRAY_SIZE(me->rx_cnt)) {
 					i = MXSER_BOARDS;
 					break;
 				}
-				port = &mxser_boards[i].ports[j];
-				if (!port->ioaddr)
+				ip = &mxser_boards[i].ports[j];
+				port = &ip->port;
+
+				mutex_lock(&port->mutex);
+				if (!ip->ioaddr) {
+					mutex_unlock(&port->mutex);
 					continue;
+				}
 
-				status = mxser_get_msr(port->ioaddr, 0, p);
+				spin_lock_irq(&ip->slock);
+				status = mxser_get_msr(ip->ioaddr, 0, p);
 
 				if (status & UART_MSR_TERI)
-					port->icount.rng++;
+					ip->icount.rng++;
 				if (status & UART_MSR_DDSR)
-					port->icount.dsr++;
+					ip->icount.dsr++;
 				if (status & UART_MSR_DDCD)
-					port->icount.dcd++;
+					ip->icount.dcd++;
 				if (status & UART_MSR_DCTS)
-					port->icount.cts++;
+					ip->icount.cts++;
 
-				port->mon_data.modem_status = status;
-				me->rx_cnt[p] = port->mon_data.rxcnt;
-				me->tx_cnt[p] = port->mon_data.txcnt;
-				me->up_rxcnt[p] = port->mon_data.up_rxcnt;
-				me->up_txcnt[p] = port->mon_data.up_txcnt;
+				ip->mon_data.modem_status = status;
+				me->rx_cnt[p] = ip->mon_data.rxcnt;
+				me->tx_cnt[p] = ip->mon_data.txcnt;
+				me->up_rxcnt[p] = ip->mon_data.up_rxcnt;
+				me->up_txcnt[p] = ip->mon_data.up_txcnt;
 				me->modem_status[p] =
-					port->mon_data.modem_status;
-				tty = tty_port_tty_get(&port->port);
+					ip->mon_data.modem_status;
+				spin_unlock_irq(&ip->slock);
+
+				tty = tty_port_tty_get(&ip->port);
 
 				if (!tty || !tty->termios) {
-					cflag = port->normal_termios.c_cflag;
-					iflag = port->normal_termios.c_iflag;
-					me->baudrate[p] = tty_termios_baud_rate(&port->normal_termios);
+					cflag = ip->normal_termios.c_cflag;
+					iflag = ip->normal_termios.c_iflag;
+					me->baudrate[p] = tty_termios_baud_rate(&ip->normal_termios);
 				} else {
 					cflag = tty->termios->c_cflag;
 					iflag = tty->termios->c_iflag;
@@ -1604,16 +1610,15 @@ static int mxser_ioctl_special(unsigned int cmd, void __user *argp)
 				if (iflag & (IXON | IXOFF))
 					me->flowctrl[p] |= 0x0C;
 
-				if (port->type == PORT_16550A)
+				if (ip->type == PORT_16550A)
 					me->fifo[p] = 1;
 
-				opmode = inb(port->opmode_ioaddr) >>
-						((p % 4) * 2);
+				opmode = inb(ip->opmode_ioaddr)>>((p % 4) * 2);
 				opmode &= OP_MODE_MASK;
 				me->iftype[p] = opmode;
+				mutex_unlock(&port->mutex);
 			}
 		}
-		unlock_kernel();
 		if (copy_to_user(argp, me, sizeof(*me)))
 			ret = -EFAULT;
 		kfree(me);
@@ -1650,6 +1655,7 @@ static int mxser_ioctl(struct tty_struct *tty, struct file *file,
 		unsigned int cmd, unsigned long arg)
 {
 	struct mxser_port *info = tty->driver_data;
+	struct tty_port *port = &info->port;
 	struct async_icount cnow;
 	unsigned long flags;
 	void __user *argp = (void __user *)arg;
@@ -1674,20 +1680,20 @@ static int mxser_ioctl(struct tty_struct *tty, struct file *file,
 					opmode != RS422_MODE &&
 					opmode != RS485_4WIRE_MODE)
 				return -EFAULT;
-			lock_kernel();
 			mask = ModeMask[p];
 			shiftbit = p * 2;
+			spin_lock_irq(&info->slock);
 			val = inb(info->opmode_ioaddr);
 			val &= mask;
 			val |= (opmode << shiftbit);
 			outb(val, info->opmode_ioaddr);
-			unlock_kernel();
+			spin_unlock_irq(&info->slock);
 		} else {
-			lock_kernel();
 			shiftbit = p * 2;
+			spin_lock_irq(&info->slock);
 			opmode = inb(info->opmode_ioaddr) >> shiftbit;
+			spin_unlock_irq(&info->slock);
 			opmode &= OP_MODE_MASK;
-			unlock_kernel();
 			if (put_user(opmode, (int __user *)argp))
 				return -EFAULT;
 		}
@@ -1700,14 +1706,14 @@ static int mxser_ioctl(struct tty_struct *tty, struct file *file,
 
 	switch (cmd) {
 	case TIOCGSERIAL:
-		lock_kernel();
+		mutex_lock(&port->mutex);
 		retval = mxser_get_serial_info(tty, argp);
-		unlock_kernel();
+		mutex_unlock(&port->mutex);
 		return retval;
 	case TIOCSSERIAL:
-		lock_kernel();
+		mutex_lock(&port->mutex);
 		retval = mxser_set_serial_info(tty, argp);
-		unlock_kernel();
+		mutex_unlock(&port->mutex);
 		return retval;
 	case TIOCSERGETLSR:	/* Get line status register */
 		return  mxser_get_lsr_info(info, argp);
@@ -1753,31 +1759,33 @@ static int mxser_ioctl(struct tty_struct *tty, struct file *file,
 	case MOXA_HighSpeedOn:
 		return put_user(info->baud_base != 115200 ? 1 : 0, (int __user *)argp);
 	case MOXA_SDS_RSTICOUNTER:
-		lock_kernel();
+		spin_lock_irq(&info->slock);
 		info->mon_data.rxcnt = 0;
 		info->mon_data.txcnt = 0;
-		unlock_kernel();
+		spin_unlock_irq(&info->slock);
 		return 0;
 
 	case MOXA_ASPP_OQUEUE:{
 		int len, lsr;
 
-		lock_kernel();
 		len = mxser_chars_in_buffer(tty);
+		spin_lock(&info->slock);
 		lsr = inb(info->ioaddr + UART_LSR) & UART_LSR_THRE;
+		spin_unlock_irq(&info->slock);
 		len += (lsr ? 0 : 1);
-		unlock_kernel();
 
 		return put_user(len, (int __user *)argp);
 	}
 	case MOXA_ASPP_MON: {
 		int mcr, status;
 
-		lock_kernel();
+		spin_lock(&info->slock);
 		status = mxser_get_msr(info->ioaddr, 1, tty->index);
 		mxser_check_modem_status(tty, info, status);
 
 		mcr = inb(info->ioaddr + UART_MCR);
+		spin_unlock(&info->slock);
+
 		if (mcr & MOXA_MUST_MCR_XON_FLAG)
 			info->mon_data.hold_reason &= ~NPPI_NOTIFY_XOFFHOLD;
 		else
@@ -1792,7 +1800,7 @@ static int mxser_ioctl(struct tty_struct *tty, struct file *file,
 			info->mon_data.hold_reason |= NPPI_NOTIFY_CTSHOLD;
 		else
 			info->mon_data.hold_reason &= ~NPPI_NOTIFY_CTSHOLD;
-		unlock_kernel();
+
 		if (copy_to_user(argp, &info->mon_data,
 				sizeof(struct mxser_mon)))
 			return -EFAULT;
@@ -1951,6 +1959,7 @@ static void mxser_wait_until_sent(struct tty_struct *tty, int timeout)
 {
 	struct mxser_port *info = tty->driver_data;
 	unsigned long orig_jiffies, char_time;
+	unsigned long flags;
 	int lsr;
 
 	if (info->type == PORT_UNKNOWN)
@@ -1990,19 +1999,21 @@ static void mxser_wait_until_sent(struct tty_struct *tty, int timeout)
 		timeout, char_time);
 	printk("jiff=%lu...", jiffies);
 #endif
-	lock_kernel();
+	spin_lock_irqsave(&info->slock, flags);
 	while (!((lsr = inb(info->ioaddr + UART_LSR)) & UART_LSR_TEMT)) {
 #ifdef SERIAL_DEBUG_RS_WAIT_UNTIL_SENT
 		printk("lsr = %d (jiff=%lu)...", lsr, jiffies);
 #endif
+		spin_unlock_irqrestore(&info->slock, flags);
 		schedule_timeout_interruptible(char_time);
+		spin_lock_irqsave(&info->slock, flags);
 		if (signal_pending(current))
 			break;
 		if (timeout && time_after(jiffies, orig_jiffies + timeout))
 			break;
 	}
+	spin_unlock_irqrestore(&info->slock, flags);
 	set_current_state(TASK_RUNNING);
-	unlock_kernel();
 
 #ifdef SERIAL_DEBUG_RS_WAIT_UNTIL_SENT
 	printk("lsr = %d (jiff=%lu)...done\n", lsr, jiffies);
-- 
1.6.5.5

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

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ