Skip to contentSkip to conversation

cdc-acm problems

PATCH Colin Leroy 2 participants 3 messages Jan 13, 2004 – Jan 15, 2004

Subsystems (2)
USB USB ohci driver
lore
Share & tools
lore

Colin Leroy

author
#0 lore raw

Hi,

I have problems with cdc-acm killing ohci. I tried to narrow down the problem, but didn't get far.
Basically killall -HUP pppd gives (in dmesg):

drivers/usb/class/cdc-acm.c: acm_ctrl_irq - urb shutting down with status: -2 ohci_hcd 0001:01:1b.1: OHCI Unrecoverable Error, disabled ohci_hcd 0001:01:1b.1: HC died; cleaning up
usb 4-1: USB disconnect, address 2
bus usb: remove device 4-1:1.0
bus usb: remove device 4-1:1.1
bus usb: remove device 4-1

I modified cdc-acm.c according to the attached patch, and noticed there may be a buffer overflow: after applying this patch, dmesg|grep high gives:
drivers/usb/class/cdc-acm.c: databits index too high: 48 drivers/usb/class/cdc-acm.c: databits index too high: 48 drivers/usb/class/cdc-acm.c: databits index too high: 48 drivers/usb/class/cdc-acm.c: databits index too high: 48 drivers/usb/class/cdc-acm.c: databits index too high: 48

I'm on a Mac (big-endian). Maybe an endianness issue ? My patch doesn't solve any problem, but maybe exposes one.

By the way, what's the difference between cpu_to_le32p() and cpu_to_le32() ? I'm wondering because of the newline.speed = cpu_to_le32p(...) line (537).

Thanks,

Signature · 1 line
Colin

Index: drivers/usb/class/cdc-acm.c
===================================================================
RCS file: /home/cvsroot/linuxppc/drivers/usb/class/cdc-acm.c,v retrieving revision 1.1.1.1

usb/class/cdc-acm.c +9 -3
diff -u -u -r1.1.1.1 cdc-acm.c
@@ -45,7 +45,7 @@
  * Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
  */
 
-#undef DEBUG
+#define DEBUG
 
 #include <linux/kernel.h>
 #include <linux/errno.h>

@@ -530,7 +530,7 @@
 	struct termios *termios = tty->termios;
 	struct acm_line newline;
 	int newctrl = acm->ctrlout;
-
+	int offset = 0;
 	if (!ACM_READY(acm))
 		return;
 

@@ -539,7 +539,13 @@
 	newline.stopbits = termios->c_cflag & CSTOPB ? 2 : 0;
 	newline.parity = termios->c_cflag & PARENB ?
 		(termios->c_cflag & PARODD ? 1 : 2) + (termios->c_cflag & CMSPAR ? 2 : 0) : 0;
-	newline.databits = acm_tty_size[(termios->c_cflag & CSIZE) >> 4];
+	
+	offset = (termios->c_cflag & CSIZE) >> 4;
+	if (offset >= sizeof(acm_tty_size)) {
+		dbg("databits index too high: %d\n", offset);
+		offset = 3;
+	}
+	newline.databits = acm_tty_size[offset];
 
 	acm->clocal = ((termios->c_cflag & CLOCAL) != 0);
author
#1 lore raw
Quoted 5 lines

I have problems with cdc-acm killing ohci. I tried to narrow down the problem, but didn't get far.
Basically killall -HUP pppd gives (in dmesg):

Quoted 7 lines

drivers/usb/class/cdc-acm.c: acm_ctrl_irq - urb shutting down with status: -2 ohci_hcd 0001:01:1b.1: OHCI Unrecoverable Error, disabled ohci_hcd 0001:01:1b.1: HC died; cleaning up
usb 4-1: USB disconnect, address 2
bus usb: remove device 4-1:1.0
bus usb: remove device 4-1:1.1
bus usb: remove device 4-1

After having looked some more hours, it looks like acm_tty_close() unlinks urbs too soon or something like that... The attached patch fixes it for me (I don't think it's really clean, but it may help ?)

Signature · 1 line
Colin

Index: drivers/usb/class/cdc-acm.c
===================================================================
RCS file: /home/cvsroot/linuxppc/drivers/usb/class/cdc-acm.c,v retrieving revision 1.1.1.1

usb/class/cdc-acm.c +11 -4
diff -u -u -r1.1.1.1 cdc-acm.c
@@ -157,6 +157,7 @@
 	unsigned int minor;				/* acm minor number */
 	unsigned char throttle;				/* throttled by tty layer */
 	unsigned char clocal;				/* termios CLOCAL */
+	unsigned int finish_remove;			/* finish removing */
 };
 
 static struct usb_driver acm_driver;

@@ -214,8 +215,16 @@
 		goto exit;
 	}
 
-	if (!ACM_READY(acm))
+	if (!ACM_READY(acm)) {
+		if (acm->finish_remove) {
+			dbg("unlinking urbs");
+			usb_unlink_urb(acm->ctrlurb);
+			usb_unlink_urb(acm->writeurb);
+			usb_unlink_urb(acm->readurb);
+			return;			
+		}
 		goto exit;
+	}
 
 	switch (dr->bRequest) {
 

@@ -382,9 +391,7 @@
 	if (!--acm->used) {
 		if (acm->dev) {
 			acm_set_control(acm, acm->ctrlout = 0);
-			usb_unlink_urb(acm->ctrlurb);
-			usb_unlink_urb(acm->writeurb);
-			usb_unlink_urb(acm->readurb);
+			acm->finish_remove = 1;
 		} else {
 			tty_unregister_device(acm_tty_driver, acm->minor);
 			acm_table[acm->minor] = NULL;
#2 lore raw
In reply to Colin Leroy
Quoted 9 lines · Colin Leroy

Hi,

I have problems with cdc-acm killing ohci. I tried to narrow down the problem, but didn't get far.
Basically killall -HUP pppd gives (in dmesg):

drivers/usb/class/cdc-acm.c: acm_ctrl_irq - urb shutting down with status: -2 ohci_hcd 0001:01:1b.1: OHCI Unrecoverable Error, disabled ohci_hcd 0001:01:1b.1: HC died; cleaning up

The above is interesting, looks like a PCI error. I keep getting those unrecoverable errors with those new USB2 capable chips, I don't know what's going on yet. Can try this patch and tell me what it dumps ?

===== drivers/usb/host/ohci-hcd.c 1.53 vs edited =====

drivers/usb/host/ohci-hcd.c +14 -1
@@ -316,6 +316,11 @@
 	/* ASSERT:  any requests/urbs are being unlinked */
 	/* ASSERT:  nobody can be submitting urbs for this any more */
 
+	if (!HCD_IS_RUNNING (ohci->hcd.state)) {
+		ed->state = ED_IDLE;
+		finish_unlinks (ohci, 0, 0);
+	}
+
 	epnum <<= 1;
 	if (epnum != 0 && !(ep & USB_DIR_IN))
 		epnum |= 1;

@@ -571,9 +576,17 @@
 		disable (ohci);
 		ohci_err (ohci, "OHCI Unrecoverable Error, disabled\n");
 		// e.g. due to PCI Master/Target Abort
+#if 1
+		if (hcd->pdev) {
+			u16 status;
+
+			pci_read_config_word(hcd->pdev, PCI_STATUS, &status);
+			printk(KERN_ERR "OHCI PCI Status: 0x%04x\n", status);
+		}
+#endif
 
 		ohci_dump (ohci, 1);
-		hc_reset (ohci);
+       		hc_reset (ohci);
 	}
   
 	if (ints & OHCI_INTR_WDH) {

===== drivers/usb/host/ohci-pci.c 1.20 vs edited =====

drivers/usb/host/ohci-pci.c +11 -0
@@ -36,6 +36,17 @@
 	int		ret;
 
 	if (hcd->pdev) {
+#if 1
+		u16 status;
+
+		pci_read_config_word(hcd->pdev, PCI_STATUS, &status);
+		printk(KERN_ERR "OHCI PCI Status: 0x%04x\n", status);
+		if (status & 0xf900) {
+			printk(KERN_ERR "Initial error ! clearing ...\n");
+			pci_write_config_word(hcd->pdev, PCI_STATUS, status);
+		}
+#endif
+
 		ohci->hcca = pci_alloc_consistent (hcd->pdev,
 				sizeof *ohci->hcca, &ohci->hcca_dma);
 		if (!ohci->hcca)