[PATCH] USB kaweth: fix races between timeout, xmit and disconnect

USB kaweth driver update

 - always set the ASYNC_UNLINK flag for the tx urb
 - correct cancelling the tx urb under all circumstances
 - detect a running disconnect method in xmit path
 - fix a potential deadlock by using GFP_NOIO

drivers/usb/net/kaweth.c

@@ -5,17 +5,18 @@
  *     (c) 2000 Interlan Communications
  *     (c) 2000 Stephane Alnet
  *     (C) 2001 Brad Hards
+ *     (C) 2002 Oliver Neukum
  *
  *     Original author: The Zapman <zapman@interlan.net>
- *     Inspired by, and much credit goes to Michael Rothwell 
+ *     Inspired by, and much credit goes to Michael Rothwell
  *     <rothwell@interlan.net> for the test equipment, help, and patience
  *     Based off of (and with thanks to) Petko Manolov's pegaus.c driver.
- *     Also many thanks to Joel Silverman and Ed Surprenant at Kawasaki 
+ *     Also many thanks to Joel Silverman and Ed Surprenant at Kawasaki
  *     for providing the firmware and driver resources.
  *
  *     This program is free software; you can redistribute it and/or
  *     modify it under the terms of the GNU General Public License as
- *     published by the Free Software Foundation; either version 2, or 
+ *     published by the Free Software Foundation; either version 2, or
  *     (at your option) any later version.
  *
  *     This program is distributed in the hope that it will be useful,
@@ -25,7 +26,7 @@
  *
  *     You should have received a copy of the GNU General Public License
  *     along with this program; if not, write to the Free Software Foundation,
- *     Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA. 
+ *     Inc., 59 Temple Place - Suite 330, Boston, MA 02111-1307, USA.
  *
  ****************************************************************/
 
@@ -36,8 +37,8 @@
  * Fix bugs from previous two steps
  * Snoop other OSs for any tricks we're not doing
  * SMP locking
- * Reduce arbitrary timeouts 
- * Smart multicast support 
+ * Reduce arbitrary timeouts
+ * Smart multicast support
  * Temporary MAC change support
  * Tunable SOFs parameter - ioctl()?
  * Ethernet stats collection
@@ -118,21 +119,21 @@ int kaweth_internal_control_msg(struct usb_device *usb_dev, unsigned int pipe,
  *     usb_device_id
  ****************************************************************/
 static struct usb_device_id usb_klsi_table[] = {
-	{ USB_DEVICE(0x03e8, 0x0008) }, /* AOX Endpoints USB Ethernet */ 
+	{ USB_DEVICE(0x03e8, 0x0008) }, /* AOX Endpoints USB Ethernet */
 	{ USB_DEVICE(0x04bb, 0x0901) }, /* I-O DATA USB-ET/T */
-	{ USB_DEVICE(0x0506, 0x03e8) }, /* 3Com 3C19250 */ 
-	{ USB_DEVICE(0x0557, 0x2002) }, /* ATEN USB Ethernet */ 
-	{ USB_DEVICE(0x0557, 0x4000) }, /* D-Link DSB-650C */ 
+	{ USB_DEVICE(0x0506, 0x03e8) }, /* 3Com 3C19250 */
+	{ USB_DEVICE(0x0557, 0x2002) }, /* ATEN USB Ethernet */
+	{ USB_DEVICE(0x0557, 0x4000) }, /* D-Link DSB-650C */
 	{ USB_DEVICE(0x0565, 0x0002) }, /* Peracom Enet */
 	{ USB_DEVICE(0x0565, 0x0003) }, /* Optus@Home UEP1045A */
-	{ USB_DEVICE(0x0565, 0x0005) }, /* Peracom Enet2 */ 
+	{ USB_DEVICE(0x0565, 0x0005) }, /* Peracom Enet2 */
 	{ USB_DEVICE(0x05e9, 0x0008) }, /* KLSI KL5KUSB101B */
 	{ USB_DEVICE(0x05e9, 0x0009) }, /* KLSI KL5KUSB101B (Board change) */
-	{ USB_DEVICE(0x066b, 0x2202) }, /* Linksys USB10T */ 
-	{ USB_DEVICE(0x06e1, 0x0008) }, /* ADS USB-10BT */ 
-	{ USB_DEVICE(0x06e1, 0x0009) }, /* ADS USB-10BT */ 
-	{ USB_DEVICE(0x0707, 0x0100) }, /* SMC 2202USB */ 
-	{ USB_DEVICE(0x07aa, 0x0001) }, /* Correga K.K. */ 
+	{ USB_DEVICE(0x066b, 0x2202) }, /* Linksys USB10T */
+	{ USB_DEVICE(0x06e1, 0x0008) }, /* ADS USB-10BT */
+	{ USB_DEVICE(0x06e1, 0x0009) }, /* ADS USB-10BT */
+	{ USB_DEVICE(0x0707, 0x0100) }, /* SMC 2202USB */
+	{ USB_DEVICE(0x07aa, 0x0001) }, /* Correga K.K. */
 	{ USB_DEVICE(0x07b8, 0x4000) }, /* D-Link DU-E10 */
 	{ USB_DEVICE(0x0846, 0x1001) }, /* NetGear EA-101 */
 	{ USB_DEVICE(0x0846, 0x1002) }, /* NetGear EA-101 */
@@ -144,9 +145,9 @@ static struct usb_device_id usb_klsi_table[] = {
 	{ USB_DEVICE(0x10bd, 0x1427) }, /* ASANTE USB To Ethernet Adapter */
 	{ USB_DEVICE(0x1342, 0x0204) }, /* Mobility USB-Ethernet Adapter */
 	{ USB_DEVICE(0x13d2, 0x0400) }, /* Shark Pocket Adapter */
-	{ USB_DEVICE(0x1645, 0x0005) }, /* Entrega E45 */ 
-	{ USB_DEVICE(0x1645, 0x0008) }, /* Entrega USB Ethernet Adapter */ 
-	{ USB_DEVICE(0x1645, 0x8005) }, /* PortGear Ethernet Adapter */ 
+	{ USB_DEVICE(0x1645, 0x0005) }, /* Entrega E45 */
+	{ USB_DEVICE(0x1645, 0x0008) }, /* Entrega USB Ethernet Adapter */
+	{ USB_DEVICE(0x1645, 0x8005) }, /* PortGear Ethernet Adapter */
 	{ USB_DEVICE(0x2001, 0x4000) }, /* D-link DSB-650C */
 	{} /* Null terminator */
 };
@@ -200,14 +201,16 @@ struct kaweth_device
 	spinlock_t device_lock;
 
 	__u32 status;
+	int end;
+	int removed;
 
 	struct usb_device *dev;
 	struct net_device *net;
-	wait_queue_head_t control_wait;
+	wait_queue_head_t term_wait;
 
 	struct urb *rx_urb;
 	struct urb *tx_urb;
-	
+
 	__u8 firmware_buf[KAWETH_FIRMWARE_BUF_SIZE];
 	__u8 tx_buf[KAWETH_BUF_SIZE];
 	__u8 rx_buf[KAWETH_BUF_SIZE];
@@ -223,13 +226,13 @@ struct kaweth_device
  *     kaweth_control
  ****************************************************************/
 static int kaweth_control(struct kaweth_device *kaweth,
-			  unsigned int pipe, 
-			  __u8 request, 
-			  __u8 requesttype, 
-			  __u16 value, 
+			  unsigned int pipe,
+			  __u8 request,
+			  __u8 requesttype,
+			  __u16 value,
 			  __u16 index,
-			  void *data, 
-			  __u16 size, 
+			  void *data,
+			  __u16 size,
 			  int timeout)
 {
 	struct usb_ctrlrequest *dr;
@@ -247,7 +250,7 @@ static int kaweth_control(struct kaweth_device *kaweth,
 		kaweth_dbg("kmalloc() failed");
 		return -ENOMEM;
 	}
-	
+
 	dr->bRequestType= requesttype;
 	dr->bRequest = request;
 	dr->wValue = cpu_to_le16p(&value);
@@ -354,19 +357,19 @@ static int kaweth_set_receive_filter(struct kaweth_device *kaweth,
 /****************************************************************
  *     kaweth_download_firmware
  ****************************************************************/
-static int kaweth_download_firmware(struct kaweth_device *kaweth, 
-				    __u8 *data, 
+static int kaweth_download_firmware(struct kaweth_device *kaweth,
+				    __u8 *data,
 				    __u16 data_len,
 				    __u8 interrupt,
 				    __u8 type)
-{	
+{
 	if(data_len > KAWETH_FIRMWARE_BUF_SIZE)	{
 		kaweth_err("Firmware too big: %d", data_len);
 		return -ENOSPC;
 	}
-	
+
 	memcpy(kaweth->firmware_buf, data, data_len);
-	
+
 	kaweth->firmware_buf[2] = (data_len & 0xFF) - 7;
 	kaweth->firmware_buf[3] = data_len >> 8;
 	kaweth->firmware_buf[4] = type;
@@ -375,8 +378,8 @@ static int kaweth_download_firmware(struct kaweth_device *kaweth,
 	kaweth_dbg("High: %i, Low:%i", kaweth->firmware_buf[3],
 		   kaweth->firmware_buf[2]);
 
-	kaweth_dbg("Downloading firmware at %p to kaweth device at %p", 
-	    data, 
+	kaweth_dbg("Downloading firmware at %p to kaweth device at %p",
+	    data,
 	    kaweth);
 	kaweth_dbg("Firmware length: %d", data_len);
 
@@ -405,7 +408,7 @@ static int kaweth_trigger_firmware(struct kaweth_device *kaweth,
 	kaweth->firmware_buf[5] = interrupt;
 	kaweth->firmware_buf[6] = 0x00;
 	kaweth->firmware_buf[7] = 0x00;
-	
+
 	kaweth_dbg("Triggering firmware");
 
 	return kaweth_control(kaweth,
@@ -429,12 +432,12 @@ static int kaweth_reset(struct kaweth_device *kaweth)
 	kaweth_dbg("kaweth_reset(%p)", kaweth);
 	result = kaweth_control(kaweth,
 				usb_sndctrlpipe(kaweth->dev, 0),
-				USB_REQ_SET_CONFIGURATION, 
-				0, 
+				USB_REQ_SET_CONFIGURATION,
+				0,
 				kaweth->dev->config[0].bConfigurationValue,
-				0, 
-				NULL, 
-				0, 
+				0,
+				NULL,
+				0,
 				KAWETH_CONTROL_TIMEOUT);
 
 	udelay(10000);
@@ -478,10 +481,10 @@ static void kaweth_usb_receive(struct urb *urb)
 {
 	struct kaweth_device *kaweth = urb->context;
 	struct net_device *net = kaweth->net;
-	
+
 	int count = urb->actual_length;
 	int count2 = urb->transfer_buffer_length;
-			
+
 	__u16 pkt_len = le16_to_cpup((u16 *)kaweth->rx_buf);
 
 	struct sk_buff *skb;
@@ -489,12 +492,20 @@ static void kaweth_usb_receive(struct urb *urb)
 	if(kaweth->status & KAWETH_STATUS_CLOSING) {
 		return;
 	}
-	
-	if(urb->status && urb->status != -EREMOTEIO && count != 1) { 
+
+	if(unlikely(urb->status == -ECONNRESET || urb->status == -ECONNABORTED))
+	/* we are killed - set a flag and wake the disconnect handler */
+	{
+		kaweth->end = 1;
+		wake_up(&kaweth->term_wait);
+		return;
+	}
+
+	if(urb->status && urb->status != -EREMOTEIO && count != 1) {
 		kaweth_err("%s RX status: %d count: %d packet_len: %d",
-                           net->name, 
+                           net->name,
 			   urb->status,
-			   count, 
+			   count,
 			   (int)pkt_len);
 		kaweth_resubmit_rx_urb(kaweth, GFP_ATOMIC);
                 return;
@@ -508,7 +519,7 @@ static void kaweth_usb_receive(struct urb *urb)
 		        kaweth_resubmit_rx_urb(kaweth, GFP_ATOMIC);
                         return;
                 }
-		
+
 		if(!(skb = dev_alloc_skb(pkt_len+2))) {
 		        kaweth_resubmit_rx_urb(kaweth, GFP_ATOMIC);
                         return;
@@ -517,13 +528,13 @@ static void kaweth_usb_receive(struct urb *urb)
 		skb->dev = net;
 
 		eth_copy_and_sum(skb, kaweth->rx_buf + 2, pkt_len, 0);
-		
+
 		skb_put(skb, pkt_len);
 
 		skb->protocol = eth_type_trans(skb, net);
-		
+
 		netif_rx(skb);
-		
+
 		kaweth->stats.rx_packets++;
 		kaweth->stats.rx_bytes += pkt_len;
 	}
@@ -602,11 +613,18 @@ static int kaweth_start_xmit(struct sk_buff *skb, struct net_device *net)
 {
 	struct kaweth_device *kaweth = net->priv;
 	int count = skb->len;
-	
+
 	int res;
 
 	spin_lock(&kaweth->device_lock);
 
+	if (kaweth->removed) {
+	/* our device is undergoing disconnection - we bail out */
+		spin_unlock(&kaweth->device_lock);
+		dev_kfree_skb(skb);
+		return 0;
+	}
+
 	kaweth_async_set_rx_mode(kaweth);
 	netif_stop_queue(net);
 
@@ -623,15 +641,17 @@ static int kaweth_start_xmit(struct sk_buff *skb, struct net_device *net)
 		      count + 2,
 		      kaweth_usb_transmit_complete,
 		      kaweth);
+	kaweth->end = 0;
+	kaweth->tx_urb->transfer_flags |= USB_ASYNC_UNLINK;
 
 	if((res = usb_submit_urb(kaweth->tx_urb, GFP_ATOMIC)))
 	{
 		kaweth_warn("kaweth failed tx_urb %d", res);
 		kaweth->stats.tx_errors++;
-		
+
 		netif_start_queue(net);
-	} 
-	else 
+	}
+	else
 	{
 		kaweth->stats.tx_packets++;
 		kaweth->stats.tx_bytes += skb->len;
@@ -651,7 +671,7 @@ static int kaweth_start_xmit(struct sk_buff *skb, struct net_device *net)
 static void kaweth_set_rx_mode(struct net_device *net)
 {
 	struct kaweth_device *kaweth = net->priv;
-	
+
 	__u16 packet_filter_bitmap = KAWETH_PACKET_FILTER_DIRECTED |
                                      KAWETH_PACKET_FILTER_BROADCAST |
 		                     KAWETH_PACKET_FILTER_MULTICAST;
@@ -662,7 +682,7 @@ static void kaweth_set_rx_mode(struct net_device *net)
 
 	if (net->flags & IFF_PROMISC) {
 		packet_filter_bitmap |= KAWETH_PACKET_FILTER_PROMISCUOUS;
-	} 
+	}
 	else if ((net->mc_count) || (net->flags & IFF_ALLMULTI)) {
 		packet_filter_bitmap |= KAWETH_PACKET_FILTER_ALL_MULTICAST;
 	}
@@ -677,7 +697,7 @@ static void kaweth_set_rx_mode(struct net_device *net)
 static void kaweth_async_set_rx_mode(struct kaweth_device *kaweth)
 {
 	__u16 packet_filter_bitmap = kaweth->packet_filter_bitmap;
-	kaweth->packet_filter_bitmap = 0;	
+	kaweth->packet_filter_bitmap = 0;
 	if(packet_filter_bitmap == 0) return;
 
 	{
@@ -737,14 +757,14 @@ static void *kaweth_probe(
 	int result = 0;
 
 	kaweth_dbg("Kawasaki Device Probe (Device number:%d): 0x%4.4x:0x%4.4x:0x%4.4x",
-		 dev->devnum, 
-		 (int)dev->descriptor.idVendor, 
+		 dev->devnum,
+		 (int)dev->descriptor.idVendor,
 		 (int)dev->descriptor.idProduct,
 		 (int)dev->descriptor.bcdDevice);
 
 	kaweth_dbg("Device at %p", dev);
 
-	kaweth_dbg("Descriptor length: %x type: %x", 
+	kaweth_dbg("Descriptor length: %x type: %x",
 		 (int)dev->descriptor.bLength,
 		 (int)dev->descriptor.bDescriptorType);
 
@@ -757,7 +777,8 @@ static void *kaweth_probe(
 
 	kaweth->dev = dev;
 	spin_lock_init(&kaweth->device_lock);
-		
+	init_waitqueue_head(&kaweth->term_wait);
+
 	kaweth_dbg("Resetting.");
 
 	kaweth_reset(kaweth);
@@ -772,20 +793,20 @@ static void *kaweth_probe(
 	} else {
 		/* Download the firmware */
 		kaweth_info("Downloading firmware...");
-		if ((result = kaweth_download_firmware(kaweth, 
-						      kaweth_new_code, 
-						      len_kaweth_new_code, 
-						      100, 
+		if ((result = kaweth_download_firmware(kaweth,
+						      kaweth_new_code,
+						      len_kaweth_new_code,
+						      100,
 						      2)) < 0) {
 			kaweth_err("Error downloading firmware (%d)", result);
 			kfree(kaweth);
 			return NULL;
 		}
 
-		if ((result = kaweth_download_firmware(kaweth, 
-						      kaweth_new_code_fix, 
-						      len_kaweth_new_code_fix, 
-						      100, 
+		if ((result = kaweth_download_firmware(kaweth,
+						      kaweth_new_code_fix,
+						      len_kaweth_new_code_fix,
+						      100,
 						      3)) < 0) {
 			kaweth_err("Error downloading firmware fix (%d)", result);
 			kfree(kaweth);
@@ -845,7 +866,7 @@ static void *kaweth_probe(
 		 (int)kaweth->configuration.hw_addr[5]);
 
 	if(!memcmp(&kaweth->configuration.hw_addr,
-                   &bcast_addr, 
+                   &bcast_addr,
 		   sizeof(bcast_addr))) {
 		kaweth_err("Firmware not functioning properly, no net device created");
 		kfree(kaweth);
@@ -856,13 +877,13 @@ static void *kaweth_probe(
 		kaweth_dbg("Error setting URB size");
 		return kaweth;
 	}
-	
+
 	if(kaweth_set_sofs_wait(kaweth, KAWETH_SOFS_TO_WAIT) < 0) {
 		kaweth_err("Error setting SOFS wait");
 		return kaweth;
 	}
 
-	result = kaweth_set_receive_filter(kaweth, 
+	result = kaweth_set_receive_filter(kaweth,
                                            KAWETH_PACKET_FILTER_DIRECTED |
                                            KAWETH_PACKET_FILTER_BROADCAST |
                                            KAWETH_PACKET_FILTER_MULTICAST);
@@ -871,7 +892,7 @@ static void *kaweth_probe(
 		kaweth_err("Error setting receive filter");
 		return kaweth;
 	}
-	
+
 	kaweth_dbg("Initializing net device.");
 
 	kaweth->tx_urb = usb_alloc_urb(0, GFP_KERNEL);
@@ -884,17 +905,17 @@ static void *kaweth_probe(
 	}
 
 	memcpy(kaweth->net->broadcast, &bcast_addr, sizeof(bcast_addr));
-	memcpy(kaweth->net->dev_addr, 
+	memcpy(kaweth->net->dev_addr,
                &kaweth->configuration.hw_addr,
                sizeof(kaweth->configuration.hw_addr));
-	 
+
 	kaweth->net->priv = kaweth;
 	kaweth->net->open = kaweth_open;
 	kaweth->net->stop = kaweth_close;
 
 	kaweth->net->watchdog_timeo = KAWETH_TX_TIMEOUT;
 	kaweth->net->tx_timeout = kaweth_tx_timeout;
-	
+
 	kaweth->net->do_ioctl = kaweth_ioctl;
 	kaweth->net->hard_start_xmit = kaweth_start_xmit;
 	kaweth->net->set_multicast_list = kaweth_set_rx_mode;
@@ -904,7 +925,7 @@ static void *kaweth_probe(
 	memset(&kaweth->stats, 0, sizeof(kaweth->stats));
 
 	kaweth_info("kaweth interface created at %s", kaweth->net->name);
-								
+
 	kaweth_dbg("Kaweth probe returning.");
 
 	return kaweth;
@@ -923,9 +944,19 @@ static void kaweth_disconnect(struct usb_device *dev, void *ptr)
 		kaweth_warn("unregistering non-existant device");
 		return;
 	}
-	usb_unlink_urb(kaweth->tx_urb);
+
+	kaweth->removed = 1;
 	usb_unlink_urb(kaweth->rx_urb);
 
+	/* we need to wait for the urb to be cancelled, if it is active */
+	spin_lock(&kaweth->device_lock);
+	if (usb_unlink_urb(kaweth->tx_urb) == -EINPROGRESS) {
+		spin_unlock(&kaweth->device_lock);
+		wait_event(kaweth->term_wait, kaweth->end);
+	} else {
+		spin_unlock(&kaweth->device_lock);
+	}
+
 	if(kaweth->net) {
 		if(kaweth->net->flags & IFF_UP) {
 			kaweth_dbg("Closing net device");
@@ -944,7 +975,7 @@ static void kaweth_disconnect(struct usb_device *dev, void *ptr)
 
 
 // FIXME this completion stuff is a modified clone of
-// an OLD version of some stuff in usb.c ... 
+// an OLD version of some stuff in usb.c ...
 struct usb_api_data {
 	wait_queue_head_t wqh;
 	int done;
@@ -974,7 +1005,7 @@ static int usb_start_wait_urb(struct urb *urb, int timeout, int* actual_length)
 
         init_waitqueue_head(&awd.wqh);
         awd.done = 0;
-        
+
         set_current_state(TASK_INTERRUPTIBLE);
         add_wait_queue(&awd.wqh, &wait);
         urb->context = &awd;
@@ -1021,7 +1052,7 @@ int kaweth_internal_control_msg(struct usb_device *usb_dev, unsigned int pipe,
         int retv;
         int length;
 
-        urb = usb_alloc_urb(0, GFP_KERNEL);
+        urb = usb_alloc_urb(0, GFP_NOIO);
         if (!urb)
                 return -ENOMEM;