usb: gadget: f_fs: Use config_ep_by_speed()
authorJack Pham <jackp@codeaurora.org>
Thu, 25 Jan 2018 07:58:20 +0000 (23:58 -0800)
committerFelipe Balbi <felipe.balbi@linux.intel.com>
Mon, 12 Feb 2018 08:52:54 +0000 (10:52 +0200)
In commit 2bfa0719ac2a ("usb: gadget: function: f_fs: pass
companion descriptor along") there is a pointer arithmetic
bug where the comp_desc is obtained as follows:

 comp_desc = (struct usb_ss_ep_comp_descriptor *)(ds +
       USB_DT_ENDPOINT_SIZE);

Since ds is a pointer to usb_endpoint_descriptor, adding
7 to it ends up going out of bounds (7 * sizeof(struct
usb_endpoint_descriptor), which is actually 7*9 bytes) past
the SS descriptor. As a result the maxburst value will be
read incorrectly, and the UDC driver will also get a garbage
comp_desc (assuming it uses it).

Since Felipe wrote, "Eventually, f_fs.c should be converted
to use config_ep_by_speed() like all other functions, though",
let's finally do it. This allows the other usb_ep fields to
be properly populated, such as maxpacket and mult. It also
eliminates the awkward speed-based descriptor lookup since
config_ep_by_speed() does that already using the ones found
in struct usb_function.

Fixes: 2bfa0719ac2a ("usb: gadget: function: f_fs: pass companion descriptor along")
Cc: stable@vger.kernel.org
Signed-off-by: Jack Pham <jackp@codeaurora.org>
Signed-off-by: Felipe Balbi <felipe.balbi@linux.intel.com>
drivers/usb/gadget/function/f_fs.c

index 49fc589fbf58c9a5ee94360bceac45e17ac54cb6..c2592d883f67c45f980c21cfa4181f77fac14760 100644 (file)
@@ -1855,44 +1855,20 @@ static int ffs_func_eps_enable(struct ffs_function *func)
 
        spin_lock_irqsave(&func->ffs->eps_lock, flags);
        while(count--) {
-               struct usb_endpoint_descriptor *ds;
-               struct usb_ss_ep_comp_descriptor *comp_desc = NULL;
-               int needs_comp_desc = false;
-               int desc_idx;
-
-               if (ffs->gadget->speed == USB_SPEED_SUPER) {
-                       desc_idx = 2;
-                       needs_comp_desc = true;
-               } else if (ffs->gadget->speed == USB_SPEED_HIGH)
-                       desc_idx = 1;
-               else
-                       desc_idx = 0;
-
-               /* fall-back to lower speed if desc missing for current speed */
-               do {
-                       ds = ep->descs[desc_idx];
-               } while (!ds && --desc_idx >= 0);
-
-               if (!ds) {
-                       ret = -EINVAL;
-                       break;
-               }
-
                ep->ep->driver_data = ep;
-               ep->ep->desc = ds;
 
-               if (needs_comp_desc) {
-                       comp_desc = (struct usb_ss_ep_comp_descriptor *)(ds +
-                                       USB_DT_ENDPOINT_SIZE);
-                       ep->ep->maxburst = comp_desc->bMaxBurst + 1;
-                       ep->ep->comp_desc = comp_desc;
+               ret = config_ep_by_speed(func->gadget, &func->function, ep->ep);
+               if (ret) {
+                       pr_err("%s: config_ep_by_speed(%s) returned %d\n",
+                                       __func__, ep->ep->name, ret);
+                       break;
                }
 
                ret = usb_ep_enable(ep->ep);
                if (likely(!ret)) {
                        epfile->ep = ep;
-                       epfile->in = usb_endpoint_dir_in(ds);
-                       epfile->isoc = usb_endpoint_xfer_isoc(ds);
+                       epfile->in = usb_endpoint_dir_in(ep->ep->desc);
+                       epfile->isoc = usb_endpoint_xfer_isoc(ep->ep->desc);
                } else {
                        break;
                }