Greg:

This patch fixes a number of small errors in the routines that handle 
messages for root hubs:

        Fill in the extra byte if a string descriptor transfer
        asks for an odd number of bytes.

        Don't copy more than urb->transfer_buffer_length bytes.
        Use an extra internal buffer to avoid the need for lots
        of bounds checking.

        Replace strcpy by strlcpy and sprintf by snprintf to avoid 
        overflowing an internal buffer.

        Don't set urb->status without first acquiring urb->lock.

Please apply.

Alan Stern



Signed-off-by: Alan Stern <[EMAIL PROTECTED]>

===== drivers/usb/core/hcd.c 1.183 vs edited =====
--- 1.183/drivers/usb/core/hcd.c        2005-01-14 18:18:16 -05:00
+++ edited/drivers/usb/core/hcd.c       2005-01-31 15:15:25 -05:00
@@ -45,6 +45,7 @@
 
 #include "usb.h"
 #include "hcd.h"
+#include "hub.h"
 
 
 // #define USB_BANDWIDTH_MESSAGES
@@ -271,6 +272,10 @@
                *utf++ = *s++;
                *utf++ = 0;
        }
+       if (utfmax > 0) {
+               *utf = *s;
+               ++retval;
+       }
        return retval;
 }
 
@@ -295,30 +300,40 @@
 
        // language ids
        if (id == 0) {
-               *data++ = 4; *data++ = 3;       /* 4 bytes string data */
-               *data++ = 0x09; *data++ = 0x04; /* MSFT-speak for "en-us" */
-               return 4;
+               buf[0] = 4;    buf[1] = 3;      /* 4 bytes string data */
+               buf[2] = 0x09; buf[3] = 0x04;   /* MSFT-speak for "en-us" */
+               len = min (len, 4);
+               memcpy (data, buf, len);
+               return len;
 
        // serial number
        } else if (id == 1) {
-               strcpy (buf, hcd->self.bus_name);
+               strlcpy (buf, hcd->self.bus_name, sizeof buf);
 
        // product description
        } else if (id == 2) {
-                strcpy (buf, hcd->product_desc);
+               strlcpy (buf, hcd->product_desc, sizeof buf);
 
        // id 3 == vendor description
        } else if (id == 3) {
-                sprintf (buf, "%s %s %s", UTS_SYSNAME, UTS_RELEASE,
-                       hcd->driver->description);
+               snprintf (buf, sizeof buf, "%s %s %s",
+                       UTS_SYSNAME, UTS_RELEASE, hcd->driver->description);
 
        // unsupported IDs --> "protocol stall"
        } else
-           return 0;
+               return -EPIPE;
 
-       data [0] = 2 * (strlen (buf) + 1);
-       data [1] = 3;   /* type == string */
-       return 2 + ascii2utf (buf, data + 2, len - 2);
+       switch (len) {          /* All cases fall through */
+       default:
+               len = 2 + ascii2utf (buf, data + 2, len - 2);
+       case 2:
+               data [1] = 3;   /* type == string */
+       case 1:
+               data [0] = 2 * (strlen (buf) + 1);
+       case 0:
+               ;               /* Compiler wants a statement here */
+       }
+       return len;
 }
 
 
@@ -327,11 +342,14 @@
 {
        struct usb_ctrlrequest *cmd;
        u16             typeReq, wValue, wIndex, wLength;
-       const u8        *bufp = NULL;
        u8              *ubuf = urb->transfer_buffer;
+       u8              tbuf [sizeof (struct usb_hub_descriptor)];
+       const u8        *bufp = tbuf;
        int             len = 0;
        int             patch_wakeup = 0;
        unsigned long   flags;
+       int             status = 0;
+       int             n;
 
        cmd = (struct usb_ctrlrequest *) urb->setup_packet;
        typeReq  = (cmd->bRequestType << 8) | cmd->bRequest;
@@ -342,17 +360,16 @@
        if (wLength > urb->transfer_buffer_length)
                goto error;
 
-       /* set up for success */
-       urb->status = 0;
-       urb->actual_length = wLength;
+       urb->actual_length = 0;
        switch (typeReq) {
 
        /* DEVICE REQUESTS */
 
        case DeviceRequest | USB_REQ_GET_STATUS:
-               ubuf [0] = (hcd->remote_wakeup << USB_DEVICE_REMOTE_WAKEUP)
+               tbuf [0] = (hcd->remote_wakeup << USB_DEVICE_REMOTE_WAKEUP)
                                | (1 << USB_DEVICE_SELF_POWERED);
-               ubuf [1] = 0;
+               tbuf [1] = 0;
+               len = 2;
                break;
        case DeviceOutRequest | USB_REQ_CLEAR_FEATURE:
                if (wValue == USB_DEVICE_REMOTE_WAKEUP)
@@ -367,7 +384,8 @@
                        goto error;
                break;
        case DeviceRequest | USB_REQ_GET_CONFIGURATION:
-               ubuf [0] = 1;
+               tbuf [0] = 1;
+               len = 1;
                        /* FALLTHROUGH */
        case DeviceOutRequest | USB_REQ_SET_CONFIGURATION:
                break;
@@ -394,16 +412,18 @@
                                patch_wakeup = 1;
                        break;
                case USB_DT_STRING << 8:
-                       urb->actual_length = rh_string (
-                               wValue & 0xff, hcd,
-                               ubuf, wLength);
+                       n = rh_string (wValue & 0xff, hcd, ubuf, wLength);
+                       if (n < 0)
+                               goto error;
+                       urb->actual_length = n;
                        break;
                default:
                        goto error;
                }
                break;
        case DeviceRequest | USB_REQ_GET_INTERFACE:
-               ubuf [0] = 0;
+               tbuf [0] = 0;
+               len = 1;
                        /* FALLTHROUGH */
        case DeviceOutRequest | USB_REQ_SET_INTERFACE:
                break;
@@ -419,8 +439,9 @@
 
        case EndpointRequest | USB_REQ_GET_STATUS:
                // ENDPOINT_HALT flag
-               ubuf [0] = 0;
-               ubuf [1] = 0;
+               tbuf [0] = 0;
+               tbuf [1] = 0;
+               len = 2;
                        /* FALLTHROUGH */
        case EndpointOutRequest | USB_REQ_CLEAR_FEATURE:
        case EndpointOutRequest | USB_REQ_SET_FEATURE:
@@ -432,19 +453,30 @@
        default:
                /* non-generic request */
                if (HCD_IS_SUSPENDED (hcd->state))
-                       urb->status = -EAGAIN;
-               else
-                       urb->status = hcd->driver->hub_control (hcd,
+                       status = -EAGAIN;
+               else {
+                       switch (typeReq) {
+                       case GetHubStatus:
+                       case GetPortStatus:
+                               len = 4;
+                               break;
+                       case GetHubDescriptor:
+                               len = sizeof (struct usb_hub_descriptor);
+                               break;
+                       }
+                       status = hcd->driver->hub_control (hcd,
                                typeReq, wValue, wIndex,
-                               ubuf, wLength);
+                               tbuf, wLength);
+               }
                break;
 error:
                /* "protocol stall" on error */
-               urb->status = -EPIPE;
+               status = -EPIPE;
        }
-       if (urb->status) {
-               urb->actual_length = 0;
-               if (urb->status != -EPIPE) {
+
+       if (status) {
+               len = 0;
+               if (status != -EPIPE) {
                        dev_dbg (hcd->self.controller,
                                "CTRL: TypeReq=0x%x val=0x%x "
                                "idx=0x%x len=%d ==> %d\n",
@@ -452,7 +484,7 @@
                                wLength, urb->status);
                }
        }
-       if (bufp) {
+       if (len) {
                if (urb->transfer_buffer_length < len)
                        len = urb->transfer_buffer_length;
                urb->actual_length = len;
@@ -460,13 +492,19 @@
                memcpy (ubuf, bufp, len);
 
                /* report whether RH hardware supports remote wakeup */
-               if (patch_wakeup)
+               if (patch_wakeup &&
+                               len > offsetof (struct usb_config_descriptor,
+                                               bmAttributes))
                        ((struct usb_config_descriptor *)ubuf)->bmAttributes
                                |= USB_CONFIG_ATT_WAKEUP;
        }
 
        /* any errors get returned through the urb completion */
        local_irq_save (flags);
+       spin_lock (&urb->lock);
+       if (urb->status == -EINPROGRESS)
+               urb->status = status;
+       spin_unlock (&urb->lock);
        usb_hcd_giveback_urb (hcd, urb, NULL);
        local_irq_restore (flags);
        return 0;



-------------------------------------------------------
This SF.Net email is sponsored by: IntelliVIEW -- Interactive Reporting
Tool for open source databases. Create drag-&-drop reports. Save time
by over 75%! Publish reports on the web. Export to DOC, XLS, RTF, etc.
Download a FREE copy at http://www.intelliview.com/go/osdn_nl
_______________________________________________
linux-usb-devel@lists.sourceforge.net
To unsubscribe, use the last form field at:
https://lists.sourceforge.net/lists/listinfo/linux-usb-devel

Reply via email to