[bug report] usb: dwc2: host: fix logical omissions in dwc2_process_non_isoc_desc

[bug report] usb: dwc2: host: fix logical omissions in dwc2_process_non_isoc_desc

[Dan Carpenter]

[ Ancient code...  -dan ]

Hello Vardan Mikayelyan,

Commit 3142a16b9816 ("usb: dwc2: host: fix logical omissions in
dwc2_process_non_isoc_desc") from Feb 16, 2016 (linux-next), leads to
the following Smatch static checker warning:

	drivers/usb/dwc2/hcd_ddma.c:1224 dwc2_complete_non_isoc_xfer_ddma()
	warn: passing freed memory 'qtd' (line 1224)

drivers/usb/dwc2/hcd_ddma.c
    1198 static void dwc2_complete_non_isoc_xfer_ddma(struct dwc2_hsotg *hsotg,
    1199                                              struct dwc2_host_chan *chan,
    1200                                              int chnum,
    1201                                              enum dwc2_halt_status halt_status)
    1202 {
    1203         struct list_head *qtd_item, *qtd_tmp;
    1204         struct dwc2_qh *qh = chan->qh;
    1205         struct dwc2_qtd *qtd = NULL;
    1206         int xfer_done;
    1207         int desc_num = 0;
    1208 
    1209         if (chan->halt_status == DWC2_HC_XFER_URB_DEQUEUE) {
    1210                 list_for_each_entry(qtd, &qh->qtd_list, qtd_list_entry)
    1211                         qtd->in_process = 0;
    1212                 return;
    1213         }
    1214 
    1215         list_for_each_safe(qtd_item, qtd_tmp, &qh->qtd_list) {
    1216                 int i;
    1217                 int qtd_desc_count;
    1218 
    1219                 qtd = list_entry(qtd_item, struct dwc2_qtd, qtd_list_entry);
    1220                 xfer_done = 0;
    1221                 qtd_desc_count = qtd->n_desc;
    1222 
    1223                 for (i = 0; i < qtd_desc_count; i++) {
--> 1224                         if (dwc2_process_non_isoc_desc(hsotg, chan, chnum, qtd,
    1225                                                        desc_num, halt_status,
    1226                                                        &xfer_done)) {

Originally dwc2_process_non_isoc_desc() would free qtd and return 1
for failed.  Not it frees it and then when this loop iterates the
next time it is a use after free.

    1227                                 qtd = NULL;
    1228                                 goto stop_scan;
    1229                         }
    1230 
    1231                         desc_num++;

Maybe add a check here on &xfer_done?

				if (xfer_done)
					break or something?

    1232                 }
    1233         }

Here is the problem in dwc2_process_non_isoc_desc()

drivers/usb/dwc2/hcd_ddma.c
  1155          failed = dwc2_update_non_isoc_urb_state_ddma(hsotg, chan, qtd, dma_desc,
  1156                                                       halt_status, n_bytes,
  1157                                                       xfer_done);
  1158          if (failed || (*xfer_done && urb->status != -EINPROGRESS)) {
  1159                  dwc2_host_complete(hsotg, qtd, urb->status);
  1160                  dwc2_hcd_qtd_unlink_and_free(hsotg, qtd, qh);
                        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
Free.

  1161                  dev_vdbg(hsotg->dev, "failed=%1x xfer_done=%1x\n",
  1162                           failed, *xfer_done);
  1163                  return failed;
                        ^^^^^^^^^^^^^
Either 1 or zero.

  1164          }

This email is a free service from the Smatch-CI project [smatch.sf.net].

regards,
dan carpenter