usb: dwc2: host: Fix dereference issue in DDMA completion flow.

Commit Message

Minas Harutyunyan April 9, 2024, 10:06 a.m. UTC

Fixed variable dereference issue in DDMA completion flow.

Fixes: b258e4268850 ("usb: dwc2: host: Fix ISOC flow in DDMA mode")
CC: [e-mail]
Signed-off-by: Minas Harutyunyan <[e-mail]>
---
 drivers/usb/dwc2/hcd_ddma.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)


base-commit: d464dac47260a33add5a206fd3289ec1216e8435

Comments

Greg KH April 9, 2024, 10:10 a.m. UTC | #1

On Tue, Apr 09, 2024 at 10:06:32AM +0000, Minas Harutyunyan wrote:
> Fixed variable dereference issue in DDMA completion flow.

What do you mean by this?

> 
> Fixes: b258e4268850 ("usb: dwc2: host: Fix ISOC flow in DDMA mode")
> CC: [e-mail]
> Signed-off-by: Minas Harutyunyan <[e-mail]>

Did someone report this problem?

thanks,

greg k-h

Minas Harutyunyan April 9, 2024, 11:39 a.m. UTC | #2

Hi Greg,

On 4/9/24 14:10, Greg Kroah-Hartman wrote:
> On Tue, Apr 09, 2024 at 10:06:32AM +0000, Minas Harutyunyan wrote:
>> Fixed variable dereference issue in DDMA completion flow.
> 
> What do you mean by this?
> 
>>
>> Fixes: b258e4268850 ("usb: dwc2: host: Fix ISOC flow in DDMA mode")
>> CC: [e-mail]
>> Signed-off-by: Minas Harutyunyan <[e-mail]>
> 
> Did someone report this problem?
> 
> thanks,
> 
> greg k-h
> 

Yes, it's reported by Dan Carpenter:

email on 4/5/24, 14:09
subject: [bug report] usb: dwc2: host: Fix ISOC flow in DDMA mode

Body:
Hello Minas Harutyunyan,

This is a semi-automatic email about new static checker warnings.

Commit b258e4268850 ("usb: dwc2: host: Fix ISOC flow in DDMA mode")
from Mar 13, 2024, leads to the following Smatch complaint:

     drivers/usb/dwc2/hcd_ddma.c:874 dwc2_cmpl_host_isoc_dma_desc()
     warn: variable dereferenced before check 'qtd->urb' (see line 870)

drivers/usb/dwc2/hcd_ddma.c
    869		u16 frame_desc_idx;
    870		struct urb *usb_urb = qtd->urb->priv;
                                       ^^^^^^^^^^
The patch adds a dereference

    871		u16 remain = 0;
    872		int rc = 0;
    873	
    874		if (!qtd->urb)
                     ^^^^^^^^^
The existing check is too late now

    875			return -EINVAL;
    876	

regards,
dan carpenter

Greg KH April 9, 2024, 11:45 a.m. UTC | #3

On Tue, Apr 09, 2024 at 11:39:00AM +0000, Minas Harutyunyan wrote:
> Hi Greg,
> 
> On 4/9/24 14:10, Greg Kroah-Hartman wrote:
> > On Tue, Apr 09, 2024 at 10:06:32AM +0000, Minas Harutyunyan wrote:
> >> Fixed variable dereference issue in DDMA completion flow.
> > 
> > What do you mean by this?
> > 
> >>
> >> Fixes: b258e4268850 ("usb: dwc2: host: Fix ISOC flow in DDMA mode")
> >> CC: [e-mail]
> >> Signed-off-by: Minas Harutyunyan <[e-mail]>
> > 
> > Did someone report this problem?
> > 
> > thanks,
> > 
> > greg k-h
> > 
> 
> Yes, it's reported by Dan Carpenter:

Then please use the proper "Reported-by:" line.

thanks,

greg k-h

Patch

diff --git a/drivers/usb/dwc2/hcd_ddma.c b/drivers/usb/dwc2/hcd_ddma.c
index 79582b102c7e..994a78ad084b 100644
--- a/drivers/usb/dwc2/hcd_ddma.c
+++ b/drivers/usb/dwc2/hcd_ddma.c
@@ -867,13 +867,15 @@  static int dwc2_cmpl_host_isoc_dma_desc(struct dwc2_hsotg *hsotg,
 	struct dwc2_dma_desc *dma_desc;
 	struct dwc2_hcd_iso_packet_desc *frame_desc;
 	u16 frame_desc_idx;
-	struct urb *usb_urb = qtd->urb->priv;
+	struct urb *usb_urb;
 	u16 remain = 0;
 	int rc = 0;
 
 	if (!qtd->urb)
 		return -EINVAL;
 
+	usb_urb = qtd->urb->priv;
+
 	dma_sync_single_for_cpu(hsotg->dev, qh->desc_list_dma + (idx *
 				sizeof(struct dwc2_dma_desc)),
 				sizeof(struct dwc2_dma_desc),