2023-07-13 09:58:42

by Pawel Laszczak

Subject: [PATCH] usb: cdnsp: Fixes issue with dequeuing not queued requests

Gadget ACM while unloading module try to dequeue not queued usb
request which causes the kernel to crash.
Patch adds extra condition to check whether usb request is processed
by CDNSP driver.

cc: <[email protected]>
Fixes: 3d82904559f4 ("usb: cdnsp: cdns3 Add main part of Cadence USBSSP DRD Driver")
Signed-off-by: Pawel Laszczak <[email protected]>
---
drivers/usb/cdns3/cdnsp-gadget.c | 3 +++
1 file changed, 3 insertions(+)

diff --git a/drivers/usb/cdns3/cdnsp-gadget.c b/drivers/usb/cdns3/cdnsp-gadget.c
index fff9ec9c391f..3a30c2af0c00 100644
--- a/drivers/usb/cdns3/cdnsp-gadget.c
+++ b/drivers/usb/cdns3/cdnsp-gadget.c
@@ -1125,6 +1125,9 @@ static int cdnsp_gadget_ep_dequeue(struct usb_ep *ep,
unsigned long flags;
int ret;

+ if (request->status != -EINPROGRESS)
+ return 0;
+
if (!pep->endpoint.desc) {
dev_err(pdev->dev,
"%s: can't dequeue to disabled endpoint\n",
--
2.37.2

2023-07-14 02:21:45

by Peter Chen

Subject: Re: [PATCH] usb: cdnsp: Fixes issue with dequeuing not queued requests

On 23-07-13 04:14:29, Pawel Laszczak wrote:
> Gadget ACM while unloading module try to dequeue not queued usb
> request which causes the kernel to crash.
> Patch adds extra condition to check whether usb request is processed
> by CDNSP driver.
>

Why ACM does that?

> cc: <[email protected]>
> Fixes: 3d82904559f4 ("usb: cdnsp: cdns3 Add main part of Cadence USBSSP DRD Driver")
> Signed-off-by: Pawel Laszczak <[email protected]>
> ---
> drivers/usb/cdns3/cdnsp-gadget.c | 3 +++
> 1 file changed, 3 insertions(+)
>
> diff --git a/drivers/usb/cdns3/cdnsp-gadget.c b/drivers/usb/cdns3/cdnsp-gadget.c
> index fff9ec9c391f..3a30c2af0c00 100644
> --- a/drivers/usb/cdns3/cdnsp-gadget.c
> +++ b/drivers/usb/cdns3/cdnsp-gadget.c
> @@ -1125,6 +1125,9 @@ static int cdnsp_gadget_ep_dequeue(struct usb_ep *ep,
> unsigned long flags;
> int ret;
>
> + if (request->status != -EINPROGRESS)
> + return 0;
> +

Why not you use pending list which used at cdnsp_ep_enqueue to do this?


> if (!pep->endpoint.desc) {
> dev_err(pdev->dev,
> "%s: can't dequeue to disabled endpoint\n",
> --
> 2.37.2
>

--

Thanks,
Peter Chen

2023-07-14 09:13:49

by Pawel Laszczak

Subject: RE: [PATCH] usb: cdnsp: Fixes issue with dequeuing not queued requests

>
>On 23-07-13 04:14:29, Pawel Laszczak wrote:
>> Gadget ACM while unloading module try to dequeue not queued usb
>> request which causes the kernel to crash.
>> Patch adds extra condition to check whether usb request is processed
>> by CDNSP driver.
>>
>
>Why ACM does that?
>
>> cc: <[email protected]>
>> Fixes: 3d82904559f4 ("usb: cdnsp: cdns3 Add main part of Cadence
>> USBSSP DRD Driver")
>> Signed-off-by: Pawel Laszczak <[email protected]>
>> ---
>> drivers/usb/cdns3/cdnsp-gadget.c | 3 +++
>> 1 file changed, 3 insertions(+)
>>
>> diff --git a/drivers/usb/cdns3/cdnsp-gadget.c
>> b/drivers/usb/cdns3/cdnsp-gadget.c
>> index fff9ec9c391f..3a30c2af0c00 100644
>> --- a/drivers/usb/cdns3/cdnsp-gadget.c
>> +++ b/drivers/usb/cdns3/cdnsp-gadget.c
>> @@ -1125,6 +1125,9 @@ static int cdnsp_gadget_ep_dequeue(struct
>usb_ep *ep,
>> unsigned long flags;
>> int ret;
>>
>> + if (request->status != -EINPROGRESS)
>> + return 0;
>> +
>
>Why not you use pending list which used at cdnsp_ep_enqueue to do this?

It's just simpler and faster way - no other reasons.

Thank,
Pawel

>
>
>> if (!pep->endpoint.desc) {
>> dev_err(pdev->dev,
>> "%s: can't dequeue to disabled endpoint\n",
>> --
>> 2.37.2
>>
>
>--
>
>Thanks,
>Peter Chen

2023-07-16 12:51:54

by Peter Chen

Subject: Re: [PATCH] usb: cdnsp: Fixes issue with dequeuing not queued requests

On 23-07-14 07:19:21, Pawel Laszczak wrote:
> >
> >On 23-07-13 04:14:29, Pawel Laszczak wrote:
> >> Gadget ACM while unloading module try to dequeue not queued usb
> >> request which causes the kernel to crash.
> >> Patch adds extra condition to check whether usb request is processed
> >> by CDNSP driver.
> >>
> >
> >Why ACM does that?

Would you please explain which situation triggers it?

> >
> >> cc: <[email protected]>
> >> Fixes: 3d82904559f4 ("usb: cdnsp: cdns3 Add main part of Cadence
> >> USBSSP DRD Driver")
> >> Signed-off-by: Pawel Laszczak <[email protected]>
> >> ---
> >> drivers/usb/cdns3/cdnsp-gadget.c | 3 +++
> >> 1 file changed, 3 insertions(+)
> >>
> >> diff --git a/drivers/usb/cdns3/cdnsp-gadget.c
> >> b/drivers/usb/cdns3/cdnsp-gadget.c
> >> index fff9ec9c391f..3a30c2af0c00 100644
> >> --- a/drivers/usb/cdns3/cdnsp-gadget.c
> >> +++ b/drivers/usb/cdns3/cdnsp-gadget.c
> >> @@ -1125,6 +1125,9 @@ static int cdnsp_gadget_ep_dequeue(struct
> >usb_ep *ep,
> >> unsigned long flags;
> >> int ret;
> >>
> >> + if (request->status != -EINPROGRESS)
> >> + return 0;
> >> +
> >
> >Why not you use pending list which used at cdnsp_ep_enqueue to do this?
>
> It's just simpler and faster way - no other reasons.

Okay, get it.

--

Thanks,
Peter Chen