[PATCH] USB: patches to acm driver

  - races with urb->current, union header evaluation, DMA handling
Signed-off-By: Oliver Neukum <oliver@neukum.name>
Signed-off-by: Greg Kroah-Hartman <greg@kroah.com>

drivers/usb/class/cdc-acm.c

@@ -5,6 +5,7 @@
  * Copyright (c) 1999 Pavel Machek	<pavel@suse.cz>
  * Copyright (c) 1999 Johannes Erdfelt	<johannes@erdfelt.com>
  * Copyright (c) 2000 Vojtech Pavlik	<vojtech@suse.cz>
+ * Copyright (c) 2004 Oliver Neukum	<oliver@neukum.name>
  *
  * USB Abstract Control Model driver for USB modems and ISDN adapters
  *
@@ -60,6 +61,7 @@
 #include <asm/uaccess.h>
 #include <linux/usb.h>
 #include <asm/byteorder.h>
+#include <asm/unaligned.h>
 
 #include "cdc-acm.h"
 
@@ -108,7 +110,7 @@ static void acm_ctrl_irq(struct urb *urb, struct pt_regs *regs)
 {
 	struct acm *acm = urb->context;
 	struct usb_ctrlrequest *dr = urb->transfer_buffer;
-	unsigned char *data = (unsigned char *)(dr + 1);
+	unsigned char *data;
 	int newctrl;
 	int status;
 
@@ -130,6 +132,7 @@ static void acm_ctrl_irq(struct urb *urb, struct pt_regs *regs)
 	if (!ACM_READY(acm))
 		goto exit;
 
+	data = (unsigned char *)(dr + 1);
 	switch (dr->bRequest) {
 
 		case ACM_IRQ_NETWORK:
@@ -139,7 +142,7 @@ static void acm_ctrl_irq(struct urb *urb, struct pt_regs *regs)
 
 		case ACM_IRQ_LINE_STATE:
 
-			newctrl = le16_to_cpup((__u16 *) data);
+			newctrl = le16_to_cpu(get_unaligned((__u16 *) data));
 
 			if (acm->tty && !acm->clocal && (acm->ctrlin & ~newctrl & ACM_CTRL_DCD)) {
 				dbg("calling hangup");
@@ -172,6 +175,7 @@ static void acm_ctrl_irq(struct urb *urb, struct pt_regs *regs)
 static void acm_read_bulk(struct urb *urb, struct pt_regs *regs)
 {
 	struct acm *acm = urb->context;
+	dbg("Entering acm_read_bulk with status %d\n", urb->status);
 
 	if (!ACM_READY(acm))
 		return;
@@ -190,6 +194,7 @@ static void acm_rx_tasklet(unsigned long _acm)
 	struct tty_struct *tty = acm->tty;
 	unsigned char *data = urb->transfer_buffer;
 	int i = 0;
+	dbg("Entering acm_rx_tasklet");
 
 	if (urb->actual_length > 0 && !acm->throttle)  {
 		for (i = 0; i < urb->actual_length && !acm->throttle; i++) {
@@ -200,14 +205,20 @@ static void acm_rx_tasklet(unsigned long _acm)
 			}
 			tty_insert_flip_char(tty, data[i], 0);
 		}
+		dbg("Handed %d bytes to tty layer", i+1);
 		tty_flip_buffer_push(tty);
 	}
 
+	spin_lock(&acm->throttle_lock);
 	if (acm->throttle) {
+		dbg("Throtteling noticed");
 		memmove(data, data + i, urb->actual_length - i);
 		urb->actual_length -= i;
+		acm->resubmit_to_unthrottle = 1;
+		spin_unlock(&acm->throttle_lock);
 		return;
 	}
+	spin_unlock(&acm->throttle_lock);
 
 	urb->actual_length = 0;
 	urb->dev = acm->dev;
@@ -221,6 +232,7 @@ static void acm_rx_tasklet(unsigned long _acm)
 static void acm_write_bulk(struct urb *urb, struct pt_regs *regs)
 {
 	struct acm *acm = (struct acm *)urb->context;
+	dbg("Entering acm_write_bulk with status %d\n", urb->status);
 
 	if (!ACM_READY(acm))
 		goto out;
@@ -237,6 +249,7 @@ static void acm_softint(void *private)
 {
 	struct acm *acm = private;
 	struct tty_struct *tty = acm->tty;
+	dbg("Entering acm_softint.\n");
 	
 	if (!ACM_READY(acm))
 		return;
@@ -254,6 +267,7 @@ static void acm_softint(void *private)
 static int acm_tty_open(struct tty_struct *tty, struct file *filp)
 {
 	struct acm *acm = acm_table[tty->index];
+	dbg("Entering acm_tty_open.\n");
 
 	if (!acm || !acm->dev)
 		return -EINVAL;
@@ -327,6 +341,7 @@ static int acm_tty_write(struct tty_struct *tty, int from_user, const unsigned c
 {
 	struct acm *acm = tty->driver_data;
 	int stat;
+	dbg("Entering acm_tty_write to write %d bytes from %s space,\n", count, from_user ? "user" : "kernel");
 
 	if (!ACM_READY(acm))
 		return -EINVAL;
@@ -337,11 +352,13 @@ static int acm_tty_write(struct tty_struct *tty, int from_user, const unsigned c
 
 	count = (count > acm->writesize) ? acm->writesize : count;
 
+	dbg("Get %d bytes from %s space...", count, from_user ? "user" : "kernel");
 	if (from_user) {
-		if (copy_from_user(acm->writeurb->transfer_buffer, (void __user *)buf, count))
+		if (copy_from_user(acm->write_buffer, (void __user *)buf, count))
 			return -EFAULT;
 	} else
-		memcpy(acm->writeurb->transfer_buffer, buf, count);
+		memcpy(acm->write_buffer, buf, count);
+	dbg("  Successfully copied.\n");
 
 	acm->writeurb->transfer_buffer_length = count;
 	acm->writeurb->dev = acm->dev;
@@ -378,7 +395,9 @@ static void acm_tty_throttle(struct tty_struct *tty)
 	struct acm *acm = tty->driver_data;
 	if (!ACM_READY(acm))
 		return;
+	spin_lock_bh(&acm->throttle_lock);
 	acm->throttle = 1;
+	spin_unlock_bh(&acm->throttle_lock);
 }
 
 static void acm_tty_unthrottle(struct tty_struct *tty)
@@ -386,9 +405,13 @@ static void acm_tty_unthrottle(struct tty_struct *tty)
 	struct acm *acm = tty->driver_data;
 	if (!ACM_READY(acm))
 		return;
+	spin_lock_bh(&acm->throttle_lock);
 	acm->throttle = 0;
-	if (acm->readurb->status != -EINPROGRESS)
+	spin_unlock_bh(&acm->throttle_lock);
+	if (acm->resubmit_to_unthrottle) {
+		acm->resubmit_to_unthrottle = 0;
 		acm_read_bulk(acm->readurb, NULL);
+	}
 }
 
 static void acm_tty_break_ctl(struct tty_struct *tty, int state)
@@ -510,7 +533,11 @@ static int acm_probe (struct usb_interface *intf,
 	struct acm *acm;
 	int minor;
 	int ctrlsize,readsize;
-	char *buf;
+	u8 *buf;
+	u8 ac_management_function = 0;
+	u8 call_management_function = 0;
+	int call_interface_num = -1;
+	int data_interface_num;
 
 	if (!buffer) {
 		err("Wierd descriptor references");
@@ -531,6 +558,18 @@ static int acm_probe (struct usb_interface *intf,
 				}
 				union_header = (struct union_desc *)buffer;
 				break;
+			case CDC_COUNTRY_TYPE: /* maybe somehow export */
+				break; /* for now we ignore it */
+			case CDC_AC_MANAGEMENT_TYPE:
+				ac_management_function = buffer[3];
+				break;
+			case CDC_CALL_MANAGEMENT_TYPE:
+				call_management_function = buffer[3];
+				call_interface_num = buffer[4];
+				if ((call_management_function & 3) != 3)
+					err("This device cannot do calls on its own. It is no modem.");
+				break;
+				
 			default:
 				err("Ignoring extra header");
 				break;
@@ -546,12 +585,15 @@ static int acm_probe (struct usb_interface *intf,
 	}
 
 	control_interface = usb_ifnum_to_if(usb_dev, union_header->bMasterInterface0);
-	data_interface = usb_ifnum_to_if(usb_dev, union_header->bSlaveInterface0);
+	data_interface = usb_ifnum_to_if(usb_dev, (data_interface_num = union_header->bSlaveInterface0));
 	if (!control_interface || !data_interface) {
 		dev_dbg(&intf->dev,"no interfaces\n");
 		return -ENODEV;
 	}
 	
+	if (data_interface_num != call_interface_num)
+		dev_dbg(&intf->dev,"Seperate call control interface. That is not fully supported.");
+
 	if (usb_interface_claimed(data_interface)) { /* valid in this context */
 		dev_dbg(&intf->dev,"The data interface isn't available\n");
 		return -EBUSY;
@@ -598,7 +640,7 @@ static int acm_probe (struct usb_interface *intf,
 
 	if (!(acm = kmalloc(sizeof(struct acm), GFP_KERNEL))) {
 		dev_dbg(&intf->dev, "out of memory (acm kmalloc)\n");
-		return -ENOMEM;
+		goto alloc_fail;
 	}
 	memset(acm, 0, sizeof(struct acm));
 
@@ -609,54 +651,66 @@ static int acm_probe (struct usb_interface *intf,
 	acm->data = data_interface;
 	acm->minor = minor;
 	acm->dev = usb_dev;
-
+	acm->ctrl_caps = ac_management_function;
+	acm->ctrlsize = ctrlsize;
+	acm->readsize = readsize;
 	acm->bh.func = acm_rx_tasklet;
 	acm->bh.data = (unsigned long) acm;
 	INIT_WORK(&acm->work, acm_softint, acm);
+	spin_lock_init(&acm->throttle_lock);
 	acm->ready_for_write = 1;
 
+	buf = usb_buffer_alloc(usb_dev, ctrlsize, GFP_KERNEL, &acm->ctrl_dma);
+	if (!buf) {
+		dev_dbg(&intf->dev, "out of memory (ctrl buffer alloc)\n");
+		goto alloc_fail2;
+	}
+	acm->ctrl_buffer = buf;
+
+	buf = usb_buffer_alloc(usb_dev, readsize, GFP_KERNEL, &acm->read_dma);
+	if (!buf) {
+		dev_dbg(&intf->dev, "out of memory (read buffer alloc)\n");
+		goto alloc_fail3;
+	}
+	acm->read_buffer = buf;
 
-	if (!(buf = kmalloc(ctrlsize + readsize + acm->writesize, GFP_KERNEL))) {
-		dev_dbg(&intf->dev, "out of memory (buf kmalloc)\n");
-		kfree(acm);
-		return -ENOMEM;
+	buf = usb_buffer_alloc(usb_dev, acm->writesize, GFP_KERNEL, &acm->write_dma);
+	if (!buf) {
+		dev_dbg(&intf->dev, "out of memory (write buffer alloc)\n");
+		goto alloc_fail4;
 	}
+	acm->write_buffer = buf;	
 
 	acm->ctrlurb = usb_alloc_urb(0, GFP_KERNEL);
 	if (!acm->ctrlurb) {
 		dev_dbg(&intf->dev, "out of memory (ctrlurb kmalloc)\n");
-		kfree(acm);
-		kfree(buf);
-		return -ENOMEM;
+		goto alloc_fail5;
 	}
 	acm->readurb = usb_alloc_urb(0, GFP_KERNEL);
 	if (!acm->readurb) {
 		dev_dbg(&intf->dev, "out of memory (readurb kmalloc)\n");
-		usb_free_urb(acm->ctrlurb);
-		kfree(acm);
-		kfree(buf);
-		return -ENOMEM;
+		goto alloc_fail6;
 	}
 	acm->writeurb = usb_alloc_urb(0, GFP_KERNEL);
 	if (!acm->writeurb) {
 		dev_dbg(&intf->dev, "out of memory (writeurb kmalloc)\n");
-		usb_free_urb(acm->readurb);
-		usb_free_urb(acm->ctrlurb);
-		kfree(acm);
-		kfree(buf);
-		return -ENOMEM;
+		goto alloc_fail7;
 	}
 
 	usb_fill_int_urb(acm->ctrlurb, usb_dev, usb_rcvintpipe(usb_dev, epctrl->bEndpointAddress),
-			 buf, ctrlsize, acm_ctrl_irq, acm, epctrl->bInterval);
+			 acm->ctrl_buffer, ctrlsize, acm_ctrl_irq, acm, epctrl->bInterval);
+	acm->ctrlurb->transfer_flags |= URB_NO_TRANSFER_DMA_MAP;
+	acm->ctrlurb->transfer_dma = acm->ctrl_dma;
 
 	usb_fill_bulk_urb(acm->readurb, usb_dev, usb_rcvbulkpipe(usb_dev, epread->bEndpointAddress),
-			  buf += ctrlsize, readsize, acm_read_bulk, acm);
-	acm->readurb->transfer_flags |= URB_NO_FSBR;
+			  acm->read_buffer, readsize, acm_read_bulk, acm);
+	acm->readurb->transfer_flags |= URB_NO_FSBR | URB_NO_TRANSFER_DMA_MAP;
+	acm->readurb->transfer_dma = acm->read_dma;
 
 	usb_fill_bulk_urb(acm->writeurb, usb_dev, usb_sndbulkpipe(usb_dev, epwrite->bEndpointAddress),
-			  buf += readsize, acm->writesize, acm_write_bulk, acm);
-	acm->writeurb->transfer_flags |= URB_NO_FSBR;
+			  acm->write_buffer, acm->writesize, acm_write_bulk, acm);
+	acm->writeurb->transfer_flags |= URB_NO_FSBR | URB_NO_TRANSFER_DMA_MAP;
+	acm->writeurb->transfer_dma = acm->write_dma;
 
 	dev_info(&intf->dev, "ttyACM%d: USB ACM device\n", minor);
 
@@ -673,17 +727,34 @@ static int acm_probe (struct usb_interface *intf,
 	acm_table[minor] = acm;
 	usb_set_intfdata (intf, acm);
 	return 0;
+
+alloc_fail7:
+	usb_free_urb(acm->readurb);
+alloc_fail6:
+	usb_free_urb(acm->ctrlurb);
+alloc_fail5:
+	usb_buffer_free(usb_dev, acm->writesize, acm->write_buffer, acm->write_dma);
+alloc_fail4:
+	usb_buffer_free(usb_dev, readsize, acm->read_buffer, acm->read_dma);
+alloc_fail3:
+	usb_buffer_free(usb_dev, ctrlsize, acm->ctrl_buffer, acm->ctrl_dma);
+alloc_fail2:
+	kfree(acm);
+alloc_fail:
+	return -ENOMEM;
 }
 
 static void acm_disconnect(struct usb_interface *intf)
 {
 	struct acm *acm = usb_get_intfdata (intf);
+	struct usb_device *usb_dev = interface_to_usbdev(intf);
 
 	if (!acm || !acm->dev) {
 		dbg("disconnect on nonexisting interface");
 		return;
 	}
 
+	down(&open_sem);
 	acm->dev = NULL;
 	usb_set_intfdata (intf, NULL);
 
@@ -693,7 +764,9 @@ static void acm_disconnect(struct usb_interface *intf)
 
 	flush_scheduled_work(); /* wait for acm_softint */
 
-	kfree(acm->ctrlurb->transfer_buffer);
+	usb_buffer_free(usb_dev, acm->writesize, acm->write_buffer, acm->write_dma);
+	usb_buffer_free(usb_dev, acm->readsize, acm->read_buffer, acm->read_dma);
+	usb_buffer_free(usb_dev, acm->ctrlsize, acm->ctrl_buffer, acm->ctrl_dma);
 
 	usb_driver_release_interface(&acm_driver, acm->data);
 
@@ -704,9 +777,12 @@ static void acm_disconnect(struct usb_interface *intf)
 		usb_free_urb(acm->readurb);
 		usb_free_urb(acm->writeurb);
 		kfree(acm);
+		up(&open_sem);
 		return;
 	}
 
+	up(&open_sem);
+
 	if (acm->tty)
 		tty_hangup(acm->tty);
 }

drivers/usb/class/cdc-acm.h

@@ -86,17 +86,23 @@ struct acm {
 	struct usb_interface *data;			/* data interface */
 	struct tty_struct *tty;				/* the corresponding tty */
 	struct urb *ctrlurb, *readurb, *writeurb;	/* urbs */
+	u8 *ctrl_buffer, *read_buffer, *write_buffer;	/* buffers of urbs */
+	dma_addr_t ctrl_dma, read_dma, write_dma;	/* dma handles of buffers */
 	struct acm_line line;				/* line coding (bits, stop, parity) */
 	struct work_struct work;			/* work queue entry for line discipline waking up */
 	struct tasklet_struct bh;			/* rx processing */
+	spinlock_t throttle_lock;			/* synchronize throtteling and read callback */
 	unsigned int ctrlin;				/* input control lines (DCD, DSR, RI, break, overruns) */
 	unsigned int ctrlout;				/* output control lines (DTR, RTS) */
 	unsigned int writesize;				/* max packet size for the output bulk endpoint */
+	unsigned int readsize,ctrlsize;			/* buffer sizes for freeing */
 	unsigned int used;				/* someone has this acm's device open */
 	unsigned int minor;				/* acm minor number */
 	unsigned char throttle;				/* throttled by tty layer */
 	unsigned char clocal;				/* termios CLOCAL */
 	unsigned char ready_for_write;			/* write urb can be used */
+	unsigned char resubmit_to_unthrottle;		/* throtteling has disabled the read urb */
+	unsigned int ctrl_caps;				/* control capabilities from the class specific header */
 };
 
 /* "Union Functional Descriptor" from CDC spec 5.2.3.X */
@@ -110,6 +116,12 @@ struct union_desc {
 	/* ... and there could be other slave interfaces */
 } __attribute__ ((packed));
 
+/* class specific descriptor types */
+#define CDC_CALL_MANAGEMENT_TYPE	0x01
+#define CDC_AC_MANAGEMENT_TYPE		0x02
 #define CDC_UNION_TYPE			0x06
+#define CDC_COUNTRY_TYPE		0x07
+
 #define CDC_DATA_INTERFACE_TYPE	0x0a
 
+