NetBSD-Bugs archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index][Old Index]

kern/43475: MTK 747 A+ GPS recorder has a bogus descriptor



>Number:         43475
>Category:       kern
>Synopsis:       MTK 747 A+ GPS recorder has a bogus descriptor
>Confidential:   no
>Severity:       serious
>Priority:       low
>Responsible:    kern-bug-people
>State:          open
>Class:          sw-bug
>Submitter-Id:   net
>Arrival-Date:   Mon Jun 14 22:25:00 +0000 2010
>Originator:     Jasper Wallace
>Release:        -current
>Organization:
>Environment:
NetBSD limpit 5.99.30 NetBSD 5.99.30 (GENERIC) #0: Wed Jun  9 20:51:58 BST 2010 
 
jasper@limpit:/home/jasper/develop/netbsd/netbsd-src-and-build/tree/n.64/obj/sys/arch/amd64/compile/GENERIC
 amd64
>Description:
The MTK 747 A+ GPS recorder is a CDM/ACM usb device, but the length of the 1st 
interface descriptor is the sum of the length of all the interface descriptors, 
so any interface descriptor parser assumes there is only one interface 
descriptor and ignores all the CDC/ACM descriptors which means the driver fails 
to attach.


>How-To-Repeat:
connect a MTK A+ Gps recorder to a netbsd system, watch it fail to attach.
>Fix:
This patch also adds a check to usbdi usb_desc_iter functions to see if 
len(first item) is > sizeof(usb_interface_descriptor_t) which should make it 
eaiser to catch this kind of thing if it happens again. It does mean that is 
any driver tries to use the iter functions from part way through a descriptor 
they are going to get random warning messages from the kernel. (i.e. the change 
to usbdi may not be a brilliant idea), also it should probably be using 
something other than printf there, maybe wrap the whole usbdi thing in #ifdef 
DEBUG?

patch (hope it dosn't get mangled by the webinterface):

Index: umodem_common.c
===================================================================
RCS file: /cvsroot/src/sys/dev/usb/umodem_common.c,v
retrieving revision 1.17
diff -u -u -r1.17 umodem_common.c
--- umodem_common.c     12 Nov 2009 19:58:27 -0000      1.17
+++ umodem_common.c     14 Jun 2010 22:21:42 -0000
@@ -369,6 +369,7 @@
        const usb_cdc_cm_descriptor_t *cmd;
        const usb_cdc_acm_descriptor_t *cad;
        const usb_cdc_union_descriptor_t *cud;
+       int ret;
 
        *cm = *acm = 0;
 
@@ -399,6 +400,55 @@
                DPRINTF(("umodem_get_caps: no UNION desc\n"));
        }
 
+       ret = cmd ? cmd->bDataInterface : cud ? cud->bSlaveInterface[0] : -1;
+       if (ret != -1)
+               return ret;
+
+       DPRINTF(("%s: probe failed, main id length: %u\n", __FUNCTION__, 
id->bLength));
+
+       /* is the interface descriptor mis sized? */
+       if (id->bLength > sizeof(usb_interface_descriptor_t))
+       {
+               int pos;
+               const usb_cdc_descriptor_t *d;
+               const usb_cdc_header_descriptor_t *header;
+               const unsigned char *desc_ptr;
+
+               pos = sizeof(usb_interface_descriptor_t);
+               desc_ptr = (const unsigned char *)id;
+               
+               while (pos < id->bLength)
+               {
+                       d = (const void *)(desc_ptr + pos);
+                       DPRINTF(("len: %x type: %02x subtype: %x\n", d->bLength,
+                                               d->bDescriptorType,
+                                               d->bDescriptorSubtype));
+                       pos += d->bLength;
+                       if (d->bDescriptorType == UDESC_CS_INTERFACE)
+                       {
+                               /* if it's not an interface type we should 
probably stop probing */
+                               switch (d->bDescriptorSubtype) {
+                               case UDESCSUB_CDC_HEADER:
+                                       header = (const 
usb_cdc_header_descriptor_t *)d;
+                                       break;
+                               case UDESCSUB_CDC_CM:
+                                       cmd = (const usb_cdc_cm_descriptor_t 
*)d;
+                                       *cm = cmd->bmCapabilities;
+                                       break;
+                               case UDESCSUB_CDC_ACM:
+                                       cad = (const usb_cdc_acm_descriptor_t 
*)d;
+                                       *acm = cad->bmCapabilities;
+                                       break;
+                               case UDESCSUB_CDC_UNION:
+                                       cud = (const usb_cdc_union_descriptor_t 
*)d;
+                                       break;
+                               default:
+                                       DPRINTF(("CDC device, but descriptor 
subtype %d not known\n", d->bDescriptorSubtype));
+                               }
+                       }
+               }
+               
+       }
        return cmd ? cmd->bDataInterface : cud ? cud->bSlaveInterface[0] : -1;
 }
 
Index: usb_quirks.c
===================================================================
RCS file: /cvsroot/src/sys/dev/usb/usb_quirks.c,v
retrieving revision 1.66
diff -u -u -r1.66 usb_quirks.c
--- usb_quirks.c        14 Mar 2010 08:44:46 -0000      1.66
+++ usb_quirks.c        14 Jun 2010 22:21:42 -0000
@@ -94,6 +94,8 @@
         0x100, { UQ_ASSUME_CM_OVER_DATA }},
  { USB_VENDOR_SIEMENS2, USB_PRODUCT_SIEMENS2_MC75,
         0x000, { UQ_ASSUME_CM_OVER_DATA }},
+ { USB_VENDOR_MTK, USB_PRODUCT_MTK_APLUSGPS,
+        ANY, { UQ_ASSUME_CM_OVER_DATA }},        
  { USB_VENDOR_TELEX, USB_PRODUCT_TELEX_MIC1,       0x009, { UQ_AU_NO_FRAC }},
  { USB_VENDOR_SILICONPORTALS, USB_PRODUCT_SILICONPORTALS_YAPPHONE,
                                                    0x100, { UQ_AU_INP_ASYNC }},
Index: usbdevs
===================================================================
RCS file: /cvsroot/src/sys/dev/usb/usbdevs,v
retrieving revision 1.552
diff -u -u -r1.552 usbdevs
--- usbdevs     6 Jun 2010 23:01:05 -0000       1.552
+++ usbdevs     14 Jun 2010 22:21:44 -0000
@@ -413,6 +413,7 @@
 vendor SITECOMEU       0x0df6  Sitecom Europe
 vendor HAWKING         0x0e66  Hawking
 vendor GMATE           0x0e7e  G.Mate, Inc
+vendor MTK             0x0e8d  MTK
 vendor OTI             0x0ea0  Ours Technology
 vendor PILOTECH                0x0eaf  Pilotech
 vendor NOVATECH                0x0eb0  Nova Tech
@@ -1697,6 +1698,9 @@
 product MUSTEK MDC800          0xa800  MDC-800 digital camera
 product MUSTEK DV2000          0xc441  DV2000 digital camera
 
+/* MTK products */
+product MTK APLUSGPS           0x3329 A+ GPS Recorder
+
 /* National Instruments */
 product NI GPIB_USB_A          0xc920  GPIB-USB-A
 
Index: usbdi.c
===================================================================
RCS file: /cvsroot/src/sys/dev/usb/usbdi.c,v
retrieving revision 1.127
diff -u -u -r1.127 usbdi.c
--- usbdi.c     16 Jan 2010 17:03:03 -0000      1.127
+++ usbdi.c     14 Jun 2010 22:21:48 -0000
@@ -1129,6 +1129,7 @@
 
         iter->cur = (const uByte *)cd;
         iter->end = (const uByte *)cd + UGETW(cd->wTotalLength);
+        iter->start = 1;
 }
 
 const usb_descriptor_t *
@@ -1146,6 +1147,14 @@
                printf("usb_desc_iter_next: descriptor length = 0\n");
                return NULL;
        }
+
+       /* if the 1st descriptor is longer than 9 it could be bogus */
+       if (iter->start) {
+               if (desc->bLength > sizeof(usb_interface_descriptor_t)) {
+                       printf("%s: first descriptor has wrong length?: %d\n", 
__FUNCTION__, desc->bLength);
+                }
+                iter->start = 0;
+        }
        iter->cur += desc->bLength;
        if (iter->cur > iter->end) {
                printf("usb_desc_iter_next: descriptor length too large\n");
Index: usbdi.h
===================================================================
RCS file: /cvsroot/src/sys/dev/usb/usbdi.h,v
retrieving revision 1.79
diff -u -u -r1.79 usbdi.h
--- usbdi.h     4 Sep 2009 17:53:12 -0000       1.79
+++ usbdi.h     14 Jun 2010 22:21:48 -0000
@@ -185,6 +185,7 @@
 typedef struct {
        const uByte *cur;
        const uByte *end;
+       bool start;
 } usbd_desc_iter_t;
 void usb_desc_iter_init(usbd_device_handle, usbd_desc_iter_t *);
 const usb_descriptor_t *usb_desc_iter_next(usbd_desc_iter_t *);



Home | Main Index | Thread Index | Old Index