ChangeSet 1.1760.26.3, 2004/06/23 15:40:17-07:00, oliver@neukum.org [PATCH] USB: patches to acm driver - races with urb->current, union header evaluation, DMA handling Signed-off-By: Oliver Neukum Signed-off-by: Greg Kroah-Hartman drivers/usb/class/cdc-acm.c | 142 +++++++++++++++++++++++++++++++++----------- drivers/usb/class/cdc-acm.h | 14 ++++ 2 files changed, 122 insertions(+), 34 deletions(-) diff -Nru a/drivers/usb/class/cdc-acm.c b/drivers/usb/class/cdc-acm.c --- a/drivers/usb/class/cdc-acm.c 2004-06-29 16:26:43 -07:00 +++ b/drivers/usb/class/cdc-acm.c 2004-06-29 16:26:43 -07:00 @@ -5,6 +5,7 @@ * Copyright (c) 1999 Pavel Machek * Copyright (c) 1999 Johannes Erdfelt * Copyright (c) 2000 Vojtech Pavlik + * Copyright (c) 2004 Oliver Neukum * * USB Abstract Control Model driver for USB modems and ISDN adapters * @@ -60,6 +61,7 @@ #include #include #include +#include #include "cdc-acm.h" @@ -108,7 +110,7 @@ { 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 @@ if (!ACM_READY(acm)) goto exit; + data = (unsigned char *)(dr + 1); switch (dr->bRequest) { case ACM_IRQ_NETWORK: @@ -139,7 +142,7 @@ 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_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 @@ 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 @@ } 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_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,7 +249,8 @@ { 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 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 @@ { 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 @@ 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 @@ 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 @@ 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 @@ 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 @@ } 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,11 +585,14 @@ } 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"); @@ -598,7 +640,7 @@ 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 @@ 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; - - 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, 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; + + 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 @@ 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 @@ 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,8 +777,11 @@ 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); diff -Nru a/drivers/usb/class/cdc-acm.h b/drivers/usb/class/cdc-acm.h --- a/drivers/usb/class/cdc-acm.h 2004-06-29 16:26:43 -07:00 +++ b/drivers/usb/class/cdc-acm.h 2004-06-29 16:26:43 -07:00 @@ -86,17 +86,23 @@ 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 @@ /* ... and there could be other slave interfaces */ } __attribute__ ((packed)); -#define CDC_UNION_TYPE 0x06 +/* 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 +