On Tue, 5 Nov 2013, David Cohen wrote: > Hi Alan, > > On 11/05/2013 07:38 AM, Alan Stern wrote: > > On Tue, 5 Nov 2013, David Cohen wrote: > > > >>>> + /* > >>>> + * Controller requires buffer size to be aligned to > >>>> + * maxpacketsize of an out endpoint. > >>>> + */ > >>>> + if (gadget->quirk_ep_out_aligned_size && read) { > >>>> + /* > >>>> + * We pass 'orig_len' to usp_ep_align_maxpacketsize() > >>>> + * due to we're in a loop and 'len' may have been > >>>> + * changed. > >>>> + */ > >>>> + len = usb_ep_align_maxpacketsize(ep->ep, orig_len); > >>>> + if (data && len > data_len) { > >>>> + kfree(data); > >>>> + data = NULL; > >>>> + data_len = 0; > >>>> + } > >>>> + } > >>> > >>> Since the value of orig_len never changes, there's no point calling > >>> usb_ep_align_maxpacketsize() inside the loop. You should call it only > >>> once, before the loop starts. Once you do that, you won't need > >>> orig_len at all. > >> > >> orig_len doesn't change but ep->ep does. If USB specs say max packet > >> size won't change even if ep does, than we can call it from outside the > >> loop. > > > > I'm not too familiar with this driver. It looks like the only way > > ep->ep can change is if the endpoint gets enabled while you're sitting > > inside the wait_event_interruptible() call. > > > > In fact, the whole structure of that loop looks peculiar. Why not > > acquire the mutex first and then do everything else? > > I'm not 100% familiar with this driver too. I'd keep this change to > another patch. > > > > > Does it even make sense for ep to change? Would this change be visible > > to the host? What if the host changes the alternate setting while this > > loop is running -- does it make sense for the userspace program to > > start a read or write under one altsetting but then have the read/write > > take place under a different altsetting? > > It doesn't make sense to do so, but gadget driver allows it. If we just > ignore, it would be a security or instability issue possible to xploit > (for DWC3 and any other controller which may depend on this quirk). Maybe Michal can enlighten us. Alan Stern -- To unsubscribe from this list: send the line "unsubscribe linux-usb" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html