Messages in this thread | | | Date | Tue, 02 May 2017 15:24:40 -0500 | From | "Gustavo A. R. Silva" <> | Subject | [usb-gadget-udc] question about null check after calling phys_to_virt() function |
| |
Hello everybody,
While looking into Coverity ID 145958 I ran into the following piece of code at drivers/usb/gadget/udc/amd5536udc.c:852:
} else if (i == buf_len) { /* first td */ td = (struct udc_data_dma *)phys_to_virt( req->td_data->next); td->status = 0; } else { td = (struct udc_data_dma *)phys_to_virt(last->next); td->status = 0; }
if (td) td->bufptr = req->req.dma + i; /* assign buffer */ else break;
The issue here is that _td_ pointer is being dereferenced before null check.
After searching for calls to phys_to_virt() function, I've noticed that is not common at all to test the returned address value.
So either the null check at line 862 is not needed or a null check before each td->status = 0; needs to be added.
I think it would be good to apply a patch like the following one:
- td->status = 0; + + if (td) + td->status = 0; + else + break; } else { td = (struct udc_data_dma *)phys_to_virt(last->next); - td->status = 0; + + if (td) + td->status = 0; + else + break; }
- if (td) - td->bufptr = req->req.dma + i; /* assign buffer */ - else - break; + td->bufptr = req->req.dma + i; /* assign buffer */
What do you think?
Thanks in advance for your comments -- Gustavo A. R. Silva
| |