Messages in this thread Patch in this message | | | Subject | Re: [PATCH] usb: gadget: f_fs: Fix kernel panic for SuperSpeed | From | Jim Lin <> | Date | Mon, 25 Apr 2016 19:32:40 +0800 |
| |
On 2016年04月22日 19:52, Felipe Balbi wrote: > * PGP Signed by an unknown key > > > Hi Jim, > > Jim Lin <jilin@nvidia.com> writes: >> Android N adds os_desc_compat in v2_descriptor by init_functionfs() >> (system/core/adb/usb_linux_client.cpp) to support automatic install >> of MTP driver on Windows for USB device mode. >> >> Current __ffs_data_do_os_desc() of f_fs.c will check reserved1 field >> and return -EINVAL. >> This results in a second adb_write of usb_linux_client.cpp >> (system/core/adb/) which doesn't have ss_descriptors filled. >> Then later kernel_panic (composite.c) occurs when ss_descriptors >> as a pointer with NULL is being accessed. > where exactly in composite.c are we dereferencing a NULL pointer ? In set_config
for (; *descriptors; ++descriptors) {
Here is the link which shows reserved (at offset 1, e.g., Function Section 2) as 1. https://msdn.microsoft.com/en-us/library/windows/hardware/gg463179.aspx
> > Is this happening on set_config() ? If that's the case, why is > gadget->speed set to USB_SPEED_SUPER to start with ? Your controller > should already have negotiated highspeed which means > function_descriptors() should have returned highspeed descriptors, not a > NULL superspeed. > > Care to explain why you haven't negotiated Highspeed ? The only thing I > can think of is that you're using a Superspeed-capable peripheral > controller (dwc3?) with maximum-speed set to Superspeed, with a > Superspeed-capable cable connected to an XHCI PC, but loading a > high-speed gadget driver (which you got from Android, written with f_fs) > and this gadget doesn't tell composite that its maximum speed is > Highspeed, instead of super-speed. > > We can add a check, sure, to avoid a kernel oops; however, you should > really fix up the gadget implementation and/or set dwc3's maximum-speed > property accordingly. > > Can you check if this patch makes your scenario work while still being > fully functional ? > > diff --git a/drivers/usb/gadget/composite.c b/drivers/usb/gadget/composite.c > index de9ffd60fcfa..3d3cdc5ed20d 100644 > --- a/drivers/usb/gadget/composite.c > +++ b/drivers/usb/gadget/composite.c > @@ -66,20 +66,36 @@ function_descriptors(struct usb_function *f, > { > struct usb_descriptor_header **descriptors; > > + /* > + * NOTE: we try to help gadget drivers which might not be setting > + * max_speed appropriately. > + */ > + > switch (speed) { > case USB_SPEED_SUPER_PLUS: > descriptors = f->ssp_descriptors; > - break; > + if (descriptors) > + break; > + /* FALLTHROUGH */ > case USB_SPEED_SUPER: > descriptors = f->ss_descriptors; > - break; > + if (descriptors) > + break; > + /* FALLTHROUGH */ > case USB_SPEED_HIGH: > descriptors = f->hs_descriptors; > - break; > + if (descriptors) > + break; > + /* FALLTHROUGH */ > default: > descriptors = f->fs_descriptors; > } > > + /* > + * if we can't find any descriptors at all, then this gadget deserves to > + * Oops with a NULL pointer dereference > + */ > + > return descriptors; > } > After trying your change, no kernel panic, but SuperSpeed device (device mode) is not enumerated by ubuntu 14.04 USB 3.0 host controller as MTP device with USB3.0 cable.
--nvpublic
| |