linux-kernel - Re: [PATCH] usb: dwc3: Fix ep0 handling when getting reset while doing control transfer

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: <ba6285cb-f9ff-8047-ad53-1f4534517b66@synopsys.com>
Date:   Wed, 25 May 2022 03:52:40 +0000
From:   Thinh Nguyen <Thinh.Nguyen@...opsys.com>
To:     Mayank Rana <quic_mrana@...cinc.com>,
        "peter.chen@...nel.org" <peter.chen@...nel.org>,
        "balbi@...nel.org" <balbi@...nel.org>,
        "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>
CC:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
        John Youn <John.Youn@...opsys.com>,
        "quic_jackp@...cinc.com" <quic_jackp@...cinc.com>,
        "quic_wcheng@...cinc.com" <quic_wcheng@...cinc.com>
Subject: Re: [PATCH] usb: dwc3: Fix ep0 handling when getting reset while
 doing control transfer

Thinh Nguyen wrote:

> Mayank Rana wrote:
>> @@ -3766,6 +3768,27 @@ static void dwc3_gadget_reset_interrupt(struct dwc3 *dwc)
>>  	}
>>  
>>  	dwc3_reset_gadget(dwc);
>> +
>> +	/*
>> +	 * From SNPS databook section 8.1.2, the EP0 should be in setup
>> +	 * phase. So ensure that EP0 is in setup phase by issuing a stall
>> +	 * and restart if EP0 is not in setup phase.
>> +	 */
> 
> We should not issue End Transfer to control endpoint unless it's error
> case such as invalid direction. The databook also specify under section
> USB reset initialization to not send End Transfer to control endpoint.
> 
>> +	if (dwc->ep0state != EP0_SETUP_PHASE) {
>> +		unsigned int	dir;
>> +
>> +		dir = !!dwc->ep0_expect_in;
>> +		if (dwc->ep0state == EP0_DATA_PHASE)
>> +			dwc3_ep0_end_control_data(dwc, dwc->eps[dir]);
>> +		else
>> +			dwc3_ep0_end_control_data(dwc, dwc->eps[!dir]);

On second thought, it seems cleaner to do this though the behavior is
undefined.

Do you have the driver tracepoint log capture of this scenario? It'd be
great if I can also review it.

Thanks!
Thinh