linux-kernel - Re: [v2] usb: dwc3: Avoid waking up gadget during startxfer

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] [day] [month] [year] [list]

Message-ID: <e222bd06-fd9f-32a3-02ca-66a01cf4ab5e@quicinc.com>
Date: Mon, 19 Aug 2024 11:07:57 +0530
From: Prashanth K <quic_prashk@...cinc.com>
To: Thinh Nguyen <Thinh.Nguyen@...opsys.com>
CC: Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
        "stable@...r.kernel.org" <stable@...r.kernel.org>
Subject: Re: [v2] usb: dwc3: Avoid waking up gadget during startxfer



On 17-08-24 03:19 am, Thinh Nguyen wrote:
> On Fri, Aug 16, 2024, Prashanth K wrote:
>> When operating in High-Speed, it is observed that DSTS[USBLNKST] doesn't
>> update link state immediately after receiving the wakeup interrupt. Since
>> wakeup event handler calls the resume callbacks, there is a chance that
>> function drivers can perform an ep queue, which in turn tries to perform
>> remote wakeup from send_gadget_ep_cmd(STARTXFER). This happens because
>> DSTS[[21:18] wasn't updated to U0 yet, it's observed that the latency of
>> DSTS can be in order of milli-seconds. Hence avoid calling gadget_wakeup
>> during startxfer to prevent unnecessarily issuing remote wakeup to host.
>>
>> Fixes: c36d8e947a56 ("usb: dwc3: gadget: put link to U0 before Start Transfer")
>> Cc: <stable@...r.kernel.org>
>> Suggested-by: Thinh Nguyen <Thinh.Nguyen@...opsys.com>
>> Signed-off-by: Prashanth K <quic_prashk@...cinc.com>
>> ---
>> v2:  Refactored the patch as suggested in v1 discussion.
>>
>>  drivers/usb/dwc3/gadget.c | 24 ------------------------
>>  1 file changed, 24 deletions(-)
>>
>> diff --git a/drivers/usb/dwc3/gadget.c b/drivers/usb/dwc3/gadget.c
>> index 89fc690fdf34..3f634209c5b8 100644
>> --- a/drivers/usb/dwc3/gadget.c
>> +++ b/drivers/usb/dwc3/gadget.c
>> @@ -327,30 +327,6 @@ int dwc3_send_gadget_ep_cmd(struct dwc3_ep *dep, unsigned int cmd,
>>  			dwc3_writel(dwc->regs, DWC3_GUSB2PHYCFG(0), reg);
> 
> Can you capture the notes I provided explaining why we can issue Start
> Transfer without checking for L1/L2/U3 states on top of this function?
> 
I also thought the same initially, but didnt add since Greg usually adds
the patch link to commit, so that the discussion would be captured in
git log. I will add it on top of dwc3_send_gadget_ep_cmd and send V3,
let me know if there's any other suggestion.

Thanks,
Prashanth K