linux-kernel - RE: [PATCH] usb: cdnsp: Fix issue with detecting command completion event

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:
 <PH7PR07MB953895BB387C725E701DCC0ADD8BA@PH7PR07MB9538.namprd07.prod.outlook.com>
Date: Thu, 8 May 2025 06:44:20 +0000
From: Pawel Laszczak <pawell@...ence.com>
To: "peter.chen@...nel.org" <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>,
        "stable@...r.kernel.org" <stable@...r.kernel.org>
Subject: RE: [PATCH] usb: cdnsp: Fix issue with detecting command completion
 event

>In some cases, there is a small-time gap in which CMD_RING_BUSY can be
>cleared by controller but adding command completion event to event ring will
>be delayed. As the result driver will return error code.
>This behavior has been detected on usbtest driver (test 9) with configuration
>including ep1in/ep1out bulk and ep2in/ep2out isoc endpoint.
>Probably this gap occurred because controller was busy with adding some
>other events to event ring.
>The CMD_RING_BUSY is cleared to '0' when the Command Descriptor has
>been executed and not when command completion event has been added to
>event ring.
>
>To fix this issue for this test the small delay is sufficient less than 10us) but to
>make sure the problem doesn't happen again in the future the patch
>introduce 3 retries to check with delay about 100us before returning error
>code
>
>Fixes: 3d82904559f4 ("usb: cdnsp: cdns3 Add main part of Cadence USBSSP
>DRD Driver")
>cc: stable@...r.kernel.org
>Signed-off-by: Pawel Laszczak <pawell@...ence.com>
>---
> drivers/usb/cdns3/cdnsp-gadget.c | 18 +++++++++++++++++-
> 1 file changed, 17 insertions(+), 1 deletion(-)
>
>diff --git a/drivers/usb/cdns3/cdnsp-gadget.c b/drivers/usb/cdns3/cdnsp-
>gadget.c
>index f773518185c9..0eb11b5dd9d3 100644
>--- a/drivers/usb/cdns3/cdnsp-gadget.c
>+++ b/drivers/usb/cdns3/cdnsp-gadget.c
>@@ -547,6 +547,7 @@ int cdnsp_wait_for_cmd_compl(struct cdnsp_device
>*pdev)
> 	dma_addr_t cmd_deq_dma;
> 	union cdnsp_trb *event;
> 	u32 cycle_state;
>+	u32 retry = 3;
> 	int ret, val;
> 	u64 cmd_dma;
> 	u32  flags;
>@@ -578,8 +579,23 @@ int cdnsp_wait_for_cmd_compl(struct cdnsp_device
>*pdev)
> 		flags = le32_to_cpu(event->event_cmd.flags);
>
> 		/* Check the owner of the TRB. */
>-		if ((flags & TRB_CYCLE) != cycle_state)
>+		if ((flags & TRB_CYCLE) != cycle_state) {
>+			/*
>+			 *Give some extra time to get chance controller
>+			 * to finish command before returning error code.
>+			 * Checking CMD_RING_BUSY is not sufficient because
>+			 * this bit is cleared to '0' when the Command
>+			 * Descriptor has been executed by controller
>+			 * and not when command completion event has
>+			 * be added to event ring.
>+			 */
>+			if (retry--) {
>+				usleep_range(90, 100);

I was guided by the warning from checkpatch.pl script and changed udelay to usleep_range.
It was wrong. In this place must be used udelay. 
I will give some time linux community for commenting  and  I will send it again in a few days.

Regards,
Pawel

>+				continue;
>+			}
>+
> 			return -EINVAL;
>+		}
>
> 		cmd_dma = le64_to_cpu(event->event_cmd.cmd_trb);
>
>--
>2.43.0