linux-kernel - RE: [PATCH] usb: cdnsp: Fix incorrect status for control request

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]

Message-ID: <BYAPR07MB5381078C05112F8453C4914EDD749@BYAPR07MB5381.namprd07.prod.outlook.com>
Date:   Mon, 13 Dec 2021 13:17:53 +0000
From:   Pawel Laszczak <pawell@...ence.com>
To:     Peter Chen <peter.chen@...nel.org>
CC:     "gregkh@...uxfoundation.org" <gregkh@...uxfoundation.org>,
        "linux-usb@...r.kernel.org" <linux-usb@...r.kernel.org>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "jianhe@...arella.com" <jianhe@...arella.com>,
        "stable@...r.kernel.org" <stable@...r.kernel.org>
Subject: RE: [PATCH] usb: cdnsp: Fix incorrect status for control request

>
>On 21-12-09 12:57:31, Pawel Laszczak wrote:
>> >> From: Pawel Laszczak <pawell@...ence.com>
>> >>
>> >> Patch fixes incorrect status for control request.
>> >> Without this fix all usb_request objects were returned to upper drivers
>> >> with usb_reqest->status field set to -EINPROGRESS.
>> >>
>> >> Fixes: 3d82904559f4 ("usb: cdnsp: cdns3 Add main part of Cadence USBSSP DRD Driver")
>> >> Reported-by: Ken (Jian) He <jianhe@...arella.com>
>> >> cc: <stable@...r.kernel.org>
>> >> Signed-off-by: Pawel Laszczak <pawell@...ence.com>
>> >> ---
>> >>  drivers/usb/cdns3/cdnsp-ring.c | 2 ++
>> >>  1 file changed, 2 insertions(+)
>> >>
>> >> diff --git a/drivers/usb/cdns3/cdnsp-ring.c b/drivers/usb/cdns3/cdnsp-ring.c
>> >> index 1b1438457fb0..e8f5ecbb5c75 100644
>> >> --- a/drivers/usb/cdns3/cdnsp-ring.c
>> >> +++ b/drivers/usb/cdns3/cdnsp-ring.c
>> >> @@ -1029,6 +1029,8 @@ static void cdnsp_process_ctrl_td(struct cdnsp_device *pdev,
>> >>  		return;
>> >>  	}
>> >>
>> >> +	*status = 0;
>> >> +
>> >>  	cdnsp_finish_td(pdev, td, event, pep, status);
>> >>  }
>> >>
>> >> --
>> >I think you may move *status = 0 at the beginning of
>> >cdnsp_process_ctrl_td in case you would like to handle some error
>> >conditions during this function.
>>
>> I don't predict any other status code for control request in this place.
>> I wanted to set this status only once after completion status stage. It was the reason why I put this
>> statement at the end of function.
>
>So, you always consider there is no error for control request handling?

Not exactly. In this place driver handles only case when status stage has been completed, 
so control transfer has been finished successfully.   

Driver still take into account for  control transfer such errors as:  STALL, EINVAL and ECONNRESET but
in other parts of source code.

--

Regards,
Pawel Laszczak