linux-kernel - Re: [PATCH v2] usb: dwc3: gadget: Wait for end transfer complete before free irq

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Thu, 13 Oct 2016 15:34:54 +0800
From:   Baolin Wang <baolin.wang@...aro.org>
To:     Felipe Balbi <balbi@...nel.org>
Cc:     Greg KH <gregkh@...uxfoundation.org>,
        Mark Brown <broonie@...nel.org>,
        USB <linux-usb@...r.kernel.org>,
        LKML <linux-kernel@...r.kernel.org>
Subject: Re: [PATCH v2] usb: dwc3: gadget: Wait for end transfer complete
 before free irq

Hi,

On 13 October 2016 at 15:02, Felipe Balbi <balbi@...nel.org> wrote:
>
> Hi,
>
> Baolin Wang <baolin.wang@...aro.org> writes:
>> @@ -1742,6 +1791,17 @@ static int dwc3_gadget_stop(struct usb_gadget *g)
>>       dwc->gadget_driver      = NULL;
>>       spin_unlock_irqrestore(&dwc->lock, flags);
>>
>> +     /*
>> +      * Since the xHCI will share the same interrupt with gadget, thus when
>> +      * free the gadget irq, it will not shutdown this gadget irq line. Then
>> +      * the gadget driver can not handle the end transfer command complete
>> +      * event after free the gadget irq, which will hang the system to crash.
>> +      *
>> +      * So we should wait for the end transfer command complete event before
>> +      * free it to avoid this situation.
>> +      */
>> +     dwc3_wait_command_complete(dwc);
>
> this doesn't make sense. We have already masked all interrupts before
> getting here. We have also, already, disabled all endpoints.

No, you just mask the interrupts described in DEVTEN register, and you
did not disable the endpoint command complete event.

>
> I'm thinking this is a bug in configfs interface of Gadget API, not
> dwc3. The only reason for this to happen would be if we get to
> ->udc_stop() with endpoints still enabled.
>
> Can you check if that's the case? i.e. can you check if any endpoints
> are still enabled when we get here?

No, it is not any endpoints are still enabled. Like I said in commit
message, we will start end transfer command when disable the endpoint,
if the end transfer command complete event comes after we have freed
the gadget irq, it will trigger the interrupt line all the time to
crash the system.

-- 
Baolin.wang
Best Regards