Re: [PATCH] usb: musb: gadget: stall when SETUP packet size is invalid

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

 



While working on v2 - already figured out MUSB_CSR0_P_DATAEND bit also
needs to be set - I found a weird thing.

For debug purpose, I changed Line 803 in musb_gadget_ep0.c as
following to stall all SETUP packets.

-    if (len != 8) {
+    if (len == 8) {

Then the protocol analyzer shows total 16 CONTROL transfers, 12
GET_DESCRIPTOR, and 4 SET_ADDRESS, all have stall response from the
device. But the musb gadget received doubled of the transfers as in
the log below. Does anyone have any thought why the transfers are
doubled on musb side?

[   24.936096] musb_g_ep0_irq 831: 0 SETUP packet len 8 != 8 ?
[   24.941955] packet: 80 06 00 01 00 00 40 00
[   24.942230] musb_g_ep0_irq 831: 0 SETUP packet len 8 != 8 ?
[   24.948059] packet: 80 06 00 01 00 00 40 00
[   24.948242] musb_g_ep0_irq 831: 0 SETUP packet len 8 != 8 ?
[   24.954040] packet: 80 06 00 01 00 00 40 00
[   25.168090] musb_g_ep0_irq 831: 0 SETUP packet len 8 != 8 ?
[   25.173919] packet: 80 06 00 01 00 00 40 00
[   25.174102] musb_g_ep0_irq 831: 0 SETUP packet len 8 != 8 ?
[   25.179931] packet: 80 06 00 01 00 00 40 00
[   25.180114] musb_g_ep0_irq 831: 0 SETUP packet len 8 != 8 ?
[   25.185913] packet: 80 06 00 01 00 00 40 00
[   25.512084] musb_g_ep0_irq 831: 0 SETUP packet len 8 != 8 ?
[   25.517913] packet: 80 06 00 01 00 00 40 00
[   25.518127] musb_g_ep0_irq 831: 0 SETUP packet len 8 != 8 ?
[   25.523925] packet: 80 06 00 01 00 00 40 00
[   25.524108] musb_g_ep0_irq 831: 0 SETUP packet len 8 != 8 ?
[   25.529937] packet: 80 06 00 01 00 00 40 00
[   25.744079] musb_g_ep0_irq 831: 0 SETUP packet len 8 != 8 ?
[   25.749908] packet: 80 06 00 01 00 00 40 00
[   25.750122] musb_g_ep0_irq 831: 0 SETUP packet len 8 != 8 ?
[   25.755920] packet: 80 06 00 01 00 00 40 00
[   25.756103] musb_g_ep0_irq 831: 0 SETUP packet len 8 != 8 ?
[   25.761932] packet: 80 06 00 01 00 00 40 00
[   26.088104] musb_g_ep0_irq 831: 0 SETUP packet len 8 != 8 ?
[   26.093933] packet: 00 05 45 00 00 00 00 00
[   26.296081] musb_g_ep0_irq 831: 0 SETUP packet len 8 != 8 ?
[   26.301910] packet: 00 05 45 00 00 00 00 00
[   26.616119] musb_g_ep0_irq 831: 0 SETUP packet len 8 != 8 ?
[   26.621948] packet: 00 05 46 00 00 00 00 00
[   26.824096] musb_g_ep0_irq 831: 0 SETUP packet len 8 != 8 ?
[   26.829925] packet: 00 05 46 00 00 00 00 00

<the analyzer has no more transfers from here>

[   27.357116] musb_g_ep0_irq 831: 0 SETUP packet len 8 != 8 ?
[   27.362945] packet: 80 06 00 01 00 00 40 00
[   27.365112] musb_g_ep0_irq 831: 0 SETUP packet len 8 != 8 ?
[   27.370941] packet: 80 06 00 01 00 00 40 00
[   27.373107] musb_g_ep0_irq 831: 0 SETUP packet len 8 != 8 ?
[   27.378936] packet: 80 06 00 01 00 00 40 00
[   27.596130] musb_g_ep0_irq 831: 0 SETUP packet len 8 != 8 ?
[   27.601959] packet: 80 06 00 01 00 00 40 00
[   27.604125] musb_g_ep0_irq 831: 0 SETUP packet len 8 != 8 ?
[   27.609954] packet: 80 06 00 01 00 00 40 00
[   27.612121] musb_g_ep0_irq 831: 0 SETUP packet len 8 != 8 ?
[   27.617950] packet: 80 06 00 01 00 00 40 00
[   28.001129] musb_g_ep0_irq 831: 0 SETUP packet len 8 != 8 ?
[   28.006988] packet: 80 06 00 01 00 00 40 00
[   28.009124] musb_g_ep0_irq 831: 0 SETUP packet len 8 != 8 ?
[   28.014953] packet: 80 06 00 01 00 00 40 00
[   28.017120] musb_g_ep0_irq 831: 0 SETUP packet len 8 != 8 ?
[   28.022949] packet: 80 06 00 01 00 00 40 00
[   28.240142] musb_g_ep0_irq 831: 0 SETUP packet len 8 != 8 ?
[   28.245971] packet: 80 06 00 01 00 00 40 00
[   28.248138] musb_g_ep0_irq 831: 0 SETUP packet len 8 != 8 ?
[   28.253967] packet: 80 06 00 01 00 00 40 00
[   28.256134] musb_g_ep0_irq 831: 0 SETUP packet len 8 != 8 ?
[   28.261962] packet: 80 06 00 01 00 00 40 00
[   28.589141] musb_g_ep0_irq 831: 0 SETUP packet len 8 != 8 ?
[   28.594970] packet: 00 05 6e 00 00 00 00 00
[   28.800140] musb_g_ep0_irq 831: 0 SETUP packet len 8 != 8 ?
[   28.805969] packet: 00 05 6e 00 00 00 00 00
[   29.177154] musb_g_ep0_irq 831: 0 SETUP packet len 8 != 8 ?
[   29.182983] packet: 00 05 6f 00 00 00 00 00
[   29.388153] musb_g_ep0_irq 831: 0 SETUP packet len 8 != 8 ?
[   29.393981] packet: 00 05 6f 00 00 00 00 00

Thanks,
-Bin.

On Fri, Nov 8, 2013 at 4:29 PM, Bin Liu <binmlist@xxxxxxxxx> wrote:
> On Fri, Nov 8, 2013 at 2:42 PM, Felipe Balbi <balbi@xxxxxx> wrote:
>> On Fri, Nov 08, 2013 at 02:20:24PM -0600, Bin Liu wrote:
>>
>> no commit log == no commit ;-)
>
> I am not sure what else needs to say other than the subject. But I
> will try to make some...
>
>>
>>> Signed-off-by: Bin Liu <b-liu@xxxxxx>
>>> ---
>>>  drivers/usb/musb/musb_gadget_ep0.c | 23 ++++++++++++++++++++++-
>>>  1 file changed, 22 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/usb/musb/musb_gadget_ep0.c b/drivers/usb/musb/musb_gadget_ep0.c
>>> index 3e9ec7c..f31b9b1 100644
>>> --- a/drivers/usb/musb/musb_gadget_ep0.c
>>> +++ b/drivers/usb/musb/musb_gadget_ep0.c
>>> @@ -664,6 +664,24 @@ __acquires(musb->lock)
>>>       return retval;
>>>  }
>>>
>>> +/* dump the EP0 fifo for debug */
>>> +static void musb_g_ep0_dump_fifo(struct musb *musb, int len)
>>> +{
>>
>> I would wrap this on #ifdef DEBUG
>>
>>> +     u8 *t;
>>
>> u8 buf[20];
>>
>>> +
>>> +     if (len <= 0)
>>> +             return;
>>> +
>>> +     t = kzalloc(len, GFP_KERNEL);
>>
>> this is called from IRQ handler, right ? It can't be GFP_KERNEL. Also,
>> you might as well just have a 20 byte buffer in the stack itself. And
>> only read up to 20 bytes, if it's more than 20 bytes we really don't
>> need everything to know that the setup packet is definitely broken.
>>
>>> +     if (!t)
>>> +             return;
>>> +
>>> +     musb->ops->read_fifo(&musb->endpoints[0], len, t);
>>
>> ... read_fifo(&musb->endpointsp[0], min(len, 20), buf);
>>
>>> +     print_hex_dump(KERN_ERR, "packet: ", DUMP_PREFIX_NONE,
>>
>> please make this KERN_DEBUG instead.
>
> Agreed on all your comments. Will fix it in next version.
>
> Thanks,
> -Bin.
>
>>
>> --
>> balbi
--
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




[Index of Archives]     [Linux Media]     [Linux Input]     [Linux Audio Users]     [Yosemite News]     [Linux Kernel]     [Linux SCSI]     [Old Linux USB Devel Archive]

  Powered by Linux