hso: net driver using tty without locking

Checking tty == NULL doesn't help us unless we have a clear semantic for the locking of the tty object in the driver. Use the tty kref objects so that we can take references to the tty in the USB event handling paths. Signed-off-by: Alan Cox <alan@redhat.com> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

hso: net driver using tty without locking
Checking tty == NULL doesn't help us unless we have a clear semantic for the locking of the tty object in the driver. Use the tty kref objects so that we can take references to the tty in the USB event handling paths. Signed-off-by: Alan Cox <alan@redhat.com> Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>
Alan Cox · Linus Torvalds
1 parent 33dd474ae7
Showing 1 changed file with 42 additions and 12 deletions Side-by-side Diff
drivers/net/usb/hso.c
@@ -1015,7 +1015,7 @@
 	struct hso_serial *serial = get_serial_by_tty(tty);
 	struct ktermios *termios;
  
-	if ((!tty) || (!tty->termios) || (!serial)) {
+	if (!serial) {
 		printk(KERN_ERR "%s: no tty structures", __func__);
 		return;
 	}
  
@@ -1057,14 +1057,14 @@
 	termios->c_cflag |= CS8;	/* character size 8 bits */
  
 	/* baud rate 115200 */
-	tty_encode_baud_rate(serial->tty, 115200, 115200);
+	tty_encode_baud_rate(tty, 115200, 115200);
  
 	/*
 	 * Force low_latency on; otherwise the pushes are scheduled;
 	 * this is bad as it opens up the possibility of dropping bytes
 	 * on the floor.  We don't want to drop bytes on the floor. :)
 	 */
-	serial->tty->low_latency = 1;
+	tty->low_latency = 1;
 	return;
 }
  
@@ -1228,6 +1228,7 @@
  
 	/* sanity check */
 	if (serial == NULL || serial->magic != HSO_SERIAL_MAGIC) {
+		WARN_ON(1);
 		tty->driver_data = NULL;
 		D1("Failed to open port");
 		return -ENODEV;
  
@@ -1242,8 +1243,10 @@
 	kref_get(&serial->parent->ref);
  
 	/* setup */
+	spin_lock_irq(&serial->serial_lock);
 	tty->driver_data = serial;
-	serial->tty = tty;
+	serial->tty = tty_kref_get(tty);
+	spin_unlock_irq(&serial->serial_lock);
  
 	/* check for port already opened, if not set the termios */
 	serial->open_count++;
@@ -1285,6 +1288,10 @@
  
 	D1("Closing serial port");
  
+	/* Open failed, no close cleanup required */
+	if (serial == NULL)
+		return;
+
 	mutex_lock(&serial->parent->mutex);
 	usb_gone = serial->parent->usb_gone;
  
  
  
@@ -1297,10 +1304,13 @@
 	kref_put(&serial->parent->ref, hso_serial_ref_free);
 	if (serial->open_count <= 0) {
 		serial->open_count = 0;
-		if (serial->tty) {
+		spin_lock_irq(&serial->serial_lock);
+		if (serial->tty == tty) {
 			serial->tty->driver_data = NULL;
 			serial->tty = NULL;
+			tty_kref_put(tty);
 		}
+		spin_unlock_irq(&serial->serial_lock);
 		if (!usb_gone)
 			hso_stop_serial_device(serial->parent);
 		tasklet_kill(&serial->unthrottle_tasklet);
@@ -1653,6 +1663,7 @@
 {
 	struct hso_serial *serial = urb->context;
 	int status = urb->status;
+	struct tty_struct *tty;
  
 	/* sanity check */
 	if (!serial) {
  
  
@@ -1662,14 +1673,18 @@
  
 	spin_lock(&serial->serial_lock);
 	serial->tx_urb_used = 0;
+	tty = tty_kref_get(serial->tty);
 	spin_unlock(&serial->serial_lock);
 	if (status) {
 		log_usb_status(status, __func__);
+		tty_kref_put(tty);
 		return;
 	}
 	hso_put_activity(serial->parent);
-	if (serial->tty)
-		tty_wakeup(serial->tty);
+	if (tty) {
+		tty_wakeup(tty);
+		tty_kref_put(tty);
+	}
 	hso_kick_transmit(serial);
  
 	D1(" ");
@@ -1706,6 +1721,7 @@
 	struct hso_serial *serial = urb->context;
 	struct usb_ctrlrequest *req;
 	int status = urb->status;
+	struct tty_struct *tty;
  
 	/* sanity check */
 	if (!serial)
  
@@ -1713,9 +1729,11 @@
  
 	spin_lock(&serial->serial_lock);
 	serial->tx_urb_used = 0;
+	tty = tty_kref_get(serial->tty);
 	spin_unlock(&serial->serial_lock);
 	if (status) {
 		log_usb_status(status, __func__);
+		tty_kref_put(tty);
 		return;
 	}
  
  
  
  
  
@@ -1734,25 +1752,31 @@
 		spin_unlock(&serial->serial_lock);
 	} else {
 		hso_put_activity(serial->parent);
-		if (serial->tty)
-			tty_wakeup(serial->tty);
+		if (tty)
+			tty_wakeup(tty);
 		/* response to a write command */
 		hso_kick_transmit(serial);
 	}
+	tty_kref_put(tty);
 }
  
 /* handle RX data for serial port */
 static int put_rxbuf_data(struct urb *urb, struct hso_serial *serial)
 {
-	struct tty_struct *tty = serial->tty;
+	struct tty_struct *tty;
 	int write_length_remaining = 0;
 	int curr_write_len;
+
 	/* Sanity check */
 	if (urb == NULL || serial == NULL) {
 		D1("serial = NULL");
 		return -2;
 	}
  
+	spin_lock(&serial->serial_lock);
+	tty = tty_kref_get(serial->tty);
+	spin_unlock(&serial->serial_lock);
+
 	/* Push data to tty */
 	if (tty) {
 		write_length_remaining = urb->actual_length -
@@ -1774,6 +1798,7 @@
 		serial->curr_rx_urb_offset = 0;
 		serial->rx_urb_filled[hso_urb_to_index(serial, urb)] = 0;
 	}
+	tty_kref_put(tty);
 	return write_length_remaining;
 }
  
  
  
@@ -2786,15 +2811,20 @@
 static void hso_free_interface(struct usb_interface *interface)
 {
 	struct hso_serial *hso_dev;
+	struct tty_struct *tty;
 	int i;
  
 	for (i = 0; i < HSO_SERIAL_TTY_MINORS; i++) {
 		if (serial_table[i]
 		    && (serial_table[i]->interface == interface)) {
 			hso_dev = dev2ser(serial_table[i]);
-			if (hso_dev->tty)
-				tty_hangup(hso_dev->tty);
+			spin_lock_irq(&hso_dev->serial_lock);
+			tty = tty_kref_get(hso_dev->tty);
+			spin_unlock_irq(&hso_dev->serial_lock);
+			if (tty)
+				tty_hangup(tty);
 			mutex_lock(&hso_dev->parent->mutex);
+			tty_kref_put(tty);
 			hso_dev->parent->usb_gone = 1;
 			mutex_unlock(&hso_dev->parent->mutex);
 			kref_put(&serial_table[i]->ref, hso_serial_ref_free);
...	...	@@ -1015,7 +1015,7 @@
1015	1015	struct hso_serial *serial = get_serial_by_tty(tty);
1016	1016	struct ktermios *termios;
1017	1017
1018		- if ((!tty) \|\| (!tty->termios) \|\| (!serial)) {
	1018	+ if (!serial) {
1019	1019	printk(KERN_ERR "%s: no tty structures", __func__);
1020	1020	return;
1021	1021	}
1022	1022
...	...	@@ -1057,14 +1057,14 @@
1057	1057	termios->c_cflag \|= CS8; /* character size 8 bits */
1058	1058
1059	1059	/* baud rate 115200 */
1060		- tty_encode_baud_rate(serial->tty, 115200, 115200);
	1060	+ tty_encode_baud_rate(tty, 115200, 115200);
1061	1061
1062	1062	/*
1063	1063	* Force low_latency on; otherwise the pushes are scheduled;
1064	1064	* this is bad as it opens up the possibility of dropping bytes
1065	1065	* on the floor. We don't want to drop bytes on the floor. :)
1066	1066	*/
1067		- serial->tty->low_latency = 1;
	1067	+ tty->low_latency = 1;
1068	1068	return;
1069	1069	}
1070	1070
...	...	@@ -1228,6 +1228,7 @@
1228	1228
1229	1229	/* sanity check */
1230	1230	if (serial == NULL \|\| serial->magic != HSO_SERIAL_MAGIC) {
	1231	+ WARN_ON(1);
1231	1232	tty->driver_data = NULL;
1232	1233	D1("Failed to open port");
1233	1234	return -ENODEV;
1234	1235
...	...	@@ -1242,8 +1243,10 @@
1242	1243	kref_get(&serial->parent->ref);
1243	1244
1244	1245	/* setup */
	1246	+ spin_lock_irq(&serial->serial_lock);
1245	1247	tty->driver_data = serial;
1246		- serial->tty = tty;
	1248	+ serial->tty = tty_kref_get(tty);
	1249	+ spin_unlock_irq(&serial->serial_lock);
1247	1250
1248	1251	/* check for port already opened, if not set the termios */
1249	1252	serial->open_count++;
...	...	@@ -1285,6 +1288,10 @@
1285	1288
1286	1289	D1("Closing serial port");
1287	1290
	1291	+ /* Open failed, no close cleanup required */
	1292	+ if (serial == NULL)
	1293	+ return;
	1294	+
1288	1295	mutex_lock(&serial->parent->mutex);
1289	1296	usb_gone = serial->parent->usb_gone;
1290	1297
1291	1298
1292	1299
...	...	@@ -1297,10 +1304,13 @@
1297	1304	kref_put(&serial->parent->ref, hso_serial_ref_free);
1298	1305	if (serial->open_count <= 0) {
1299	1306	serial->open_count = 0;
1300		- if (serial->tty) {
	1307	+ spin_lock_irq(&serial->serial_lock);
	1308	+ if (serial->tty == tty) {
1301	1309	serial->tty->driver_data = NULL;
1302	1310	serial->tty = NULL;
	1311	+ tty_kref_put(tty);
1303	1312	}
	1313	+ spin_unlock_irq(&serial->serial_lock);
1304	1314	if (!usb_gone)
1305	1315	hso_stop_serial_device(serial->parent);
1306	1316	tasklet_kill(&serial->unthrottle_tasklet);
...	...	@@ -1653,6 +1663,7 @@
1653	1663	{
1654	1664	struct hso_serial *serial = urb->context;
1655	1665	int status = urb->status;
	1666	+ struct tty_struct *tty;
1656	1667
1657	1668	/* sanity check */
1658	1669	if (!serial) {
1659	1670
1660	1671
...	...	@@ -1662,14 +1673,18 @@
1662	1673
1663	1674	spin_lock(&serial->serial_lock);
1664	1675	serial->tx_urb_used = 0;
	1676	+ tty = tty_kref_get(serial->tty);
1665	1677	spin_unlock(&serial->serial_lock);
1666	1678	if (status) {
1667	1679	log_usb_status(status, __func__);
	1680	+ tty_kref_put(tty);
1668	1681	return;
1669	1682	}
1670	1683	hso_put_activity(serial->parent);
1671		- if (serial->tty)
1672		- tty_wakeup(serial->tty);
	1684	+ if (tty) {
	1685	+ tty_wakeup(tty);
	1686	+ tty_kref_put(tty);
	1687	+ }
1673	1688	hso_kick_transmit(serial);
1674	1689
1675	1690	D1(" ");
...	...	@@ -1706,6 +1721,7 @@
1706	1721	struct hso_serial *serial = urb->context;
1707	1722	struct usb_ctrlrequest *req;
1708	1723	int status = urb->status;
	1724	+ struct tty_struct *tty;
1709	1725
1710	1726	/* sanity check */
1711	1727	if (!serial)
1712	1728
...	...	@@ -1713,9 +1729,11 @@
1713	1729
1714	1730	spin_lock(&serial->serial_lock);
1715	1731	serial->tx_urb_used = 0;
	1732	+ tty = tty_kref_get(serial->tty);
1716	1733	spin_unlock(&serial->serial_lock);
1717	1734	if (status) {
1718	1735	log_usb_status(status, __func__);
	1736	+ tty_kref_put(tty);
1719	1737	return;
1720	1738	}
1721	1739
1722	1740
1723	1741
1724	1742
1725	1743
...	...	@@ -1734,25 +1752,31 @@
1734	1752	spin_unlock(&serial->serial_lock);
1735	1753	} else {
1736	1754	hso_put_activity(serial->parent);
1737		- if (serial->tty)
1738		- tty_wakeup(serial->tty);
	1755	+ if (tty)
	1756	+ tty_wakeup(tty);
1739	1757	/* response to a write command */
1740	1758	hso_kick_transmit(serial);
1741	1759	}
	1760	+ tty_kref_put(tty);
1742	1761	}
1743	1762
1744	1763	/* handle RX data for serial port */
1745	1764	static int put_rxbuf_data(struct urb urb, struct hso_serial serial)
1746	1765	{
1747		- struct tty_struct *tty = serial->tty;
	1766	+ struct tty_struct *tty;
1748	1767	int write_length_remaining = 0;
1749	1768	int curr_write_len;
	1769	+
1750	1770	/* Sanity check */
1751	1771	if (urb == NULL \|\| serial == NULL) {
1752	1772	D1("serial = NULL");
1753	1773	return -2;
1754	1774	}
1755	1775
	1776	+ spin_lock(&serial->serial_lock);
	1777	+ tty = tty_kref_get(serial->tty);
	1778	+ spin_unlock(&serial->serial_lock);
	1779	+
1756	1780	/* Push data to tty */
1757	1781	if (tty) {
1758	1782	write_length_remaining = urb->actual_length -
...	...	@@ -1774,6 +1798,7 @@
1774	1798	serial->curr_rx_urb_offset = 0;
1775	1799	serial->rx_urb_filled[hso_urb_to_index(serial, urb)] = 0;
1776	1800	}
	1801	+ tty_kref_put(tty);
1777	1802	return write_length_remaining;
1778	1803	}
1779	1804
1780	1805
1781	1806
...	...	@@ -2786,15 +2811,20 @@
2786	2811	static void hso_free_interface(struct usb_interface *interface)
2787	2812	{
2788	2813	struct hso_serial *hso_dev;
	2814	+ struct tty_struct *tty;
2789	2815	int i;
2790	2816
2791	2817	for (i = 0; i < HSO_SERIAL_TTY_MINORS; i++) {
2792	2818	if (serial_table[i]
2793	2819	&& (serial_table[i]->interface == interface)) {
2794	2820	hso_dev = dev2ser(serial_table[i]);
2795		- if (hso_dev->tty)
2796		- tty_hangup(hso_dev->tty);
	2821	+ spin_lock_irq(&hso_dev->serial_lock);
	2822	+ tty = tty_kref_get(hso_dev->tty);
	2823	+ spin_unlock_irq(&hso_dev->serial_lock);
	2824	+ if (tty)
	2825	+ tty_hangup(tty);
2797	2826	mutex_lock(&hso_dev->parent->mutex);
	2827	+ tty_kref_put(tty);
2798	2828	hso_dev->parent->usb_gone = 1;
2799	2829	mutex_unlock(&hso_dev->parent->mutex);
2800	2830	kref_put(&serial_table[i]->ref, hso_serial_ref_free);