linux-kernel - Re: [PATCH v9 04/10] usb: dwc3: core: Skip setting event buffers for host only controllers

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:   Sat, 24 Jun 2023 12:50:12 +0530
From:   Krishna Kurapati PSSNV <quic_kriskura@...cinc.com>
To:     Thinh Nguyen <Thinh.Nguyen@...opsys.com>
CC:     Greg Kroah-Hartman <gregkh@...uxfoundation.org>,
        Philipp Zabel <p.zabel@...gutronix.de>,
        Andy Gross <agross@...nel.org>,
        Bjorn Andersson <andersson@...nel.org>,
        Konrad Dybcio <konrad.dybcio@...aro.org>,
        Rob Herring <robh+dt@...nel.org>,
        Krzysztof Kozlowski <krzysztof.kozlowski+dt@...aro.org>,
        Felipe Balbi <balbi@...nel.org>,
        "Wesley Cheng" <quic_wcheng@...cinc.com>,
        Johan Hovold <johan@...nel.org>,
        "linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-arm-msm@...r.kernel.org" <linux-arm-msm@...r.kernel.org>,
        "devicetree@...r.kernel.org" <devicetree@...r.kernel.org>,
        "quic_pkondeti@...cinc.com" <quic_pkondeti@...cinc.com>,
        "quic_ppratap@...cinc.com" <quic_ppratap@...cinc.com>,
        "quic_jackp@...cinc.com" <quic_jackp@...cinc.com>,
        "quic_harshq@...cinc.com" <quic_harshq@...cinc.com>,
        "ahalaney@...hat.com" <ahalaney@...hat.com>,
        "quic_shazhuss@...cinc.com" <quic_shazhuss@...cinc.com>
Subject: Re: [PATCH v9 04/10] usb: dwc3: core: Skip setting event buffers for
 host only controllers



On 6/24/2023 3:57 AM, Thinh Nguyen wrote:
> On Wed, Jun 21, 2023, Krishna Kurapati wrote:
>> On some SoC's like SA8295P where the tertiary controller is host-only
>> capable, GEVTADDRHI/LO, GEVTSIZ, GEVTCOUNT registers are not accessible.
>> Trying to access them leads to a crash.
>>
>> For DRD/Peripheral supported controllers, event buffer setup is done
>> again in gadget_pullup. Skip setup or cleanup of event buffers if
>> controller is host-only capable.
>>
>> Suggested-by: Johan Hovold <johan@...nel.org>
>> Signed-off-by: Krishna Kurapati <quic_kriskura@...cinc.com>
>> ---
>>   drivers/usb/dwc3/core.c | 11 +++++++++++
>>   1 file changed, 11 insertions(+)
>>
>> diff --git a/drivers/usb/dwc3/core.c b/drivers/usb/dwc3/core.c
>> index 32ec05fc242b..e1ebae54454f 100644
>> --- a/drivers/usb/dwc3/core.c
>> +++ b/drivers/usb/dwc3/core.c
>> @@ -486,6 +486,11 @@ static void dwc3_free_event_buffers(struct dwc3 *dwc)
>>   static int dwc3_alloc_event_buffers(struct dwc3 *dwc, unsigned int length)
>>   {
>>   	struct dwc3_event_buffer *evt;
>> +	unsigned int hw_mode;
>> +
>> +	hw_mode = DWC3_GHWPARAMS0_MODE(dwc->hwparams.hwparams0);
>> +	if (hw_mode == DWC3_GHWPARAMS0_MODE_HOST)
>> +		return 0;
> 
> This is a little awkward. Returning 0 here indicates that this function
> was successful, and the event buffers were allocated based on the
> function name. Do this check outside of dwc3_alloc_one_event_buffer()
> and specifically set dwc->ev_buf = NULL if that's the case.
> 
Hi Thinh,

   Apologies, I didn't understand the comment properly.

   I thought we were supposed to return 0 here if we fulfill the goal of 
this function (allocate if we are drd/gadget and don't allocate if we 
are host mode only).

   If we return a non zero error here, probe would fail as this call 
will be done only for host only controllers during probe and nowhere else.

   Are you suggesting we move this check to dwc3_alloc_one_event_buffer 
call ?

Regards,
Krishna,

>>   
>>   	evt = dwc3_alloc_one_event_buffer(dwc, length);
>>   	if (IS_ERR(evt)) {
>> @@ -507,6 +512,9 @@ int dwc3_event_buffers_setup(struct dwc3 *dwc)
>>   {
>>   	struct dwc3_event_buffer	*evt;
>>   
>> +	if (!dwc->ev_buf)
>> +		return 0;
>> +
>>   	evt = dwc->ev_buf;
>>   	evt->lpos = 0;
>>   	dwc3_writel(dwc->regs, DWC3_GEVNTADRLO(0),
>> @@ -524,6 +532,9 @@ void dwc3_event_buffers_cleanup(struct dwc3 *dwc)
>>   {
>>   	struct dwc3_event_buffer	*evt;
>>   
>> +	if (!dwc->ev_buf)
>> +		return;
>> +
>>   	evt = dwc->ev_buf;
>>   
>>   	evt->lpos = 0;
>> -- 
>> 2.40.0