| Message ID | 8c64fdeb-5857-8cb3-cfd8-0c248a14b909@omp.ru (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | c6c986b657e55910746ea0f41e1aee0696833c7a |
| Headers | show |
| Series | [v2] usb: host: ehci-q: make qtd_fill() return *unsigned int* | expand |
On Thu, Feb 17, 2022 at 08:30:13PM +0300, Sergey Shtylyov wrote: > At the end of qtd_fill(), we assign the 'int count' variable to the 'size_t > length' field of 'struct ehci_qtd' -- which implies a problematic type cast. > Let's make that variable and the function's result *unsigned int* instead... > > Found by Linux Verification Center (linuxtesting.org) with the SVACE static > analysis tool. > > Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru> > > --- Acked-by: Alan Stern <stern@rowland.harvard.edu> > This patch is against the 'usb-next' branch of Greg KH's 'usb.git' repo. > > Changes in version 2: > - changed 'u16' to *unsigned int* everywhere, repordering the declarations; > - updated the type of the 'this_qtd_len' local variable qh_urb_transaction(); > - rewrote the patch description. > > drivers/usb/host/ehci-q.c | 7 ++++--- > 1 file changed, 4 insertions(+), 3 deletions(-) > > Index: usb/drivers/usb/host/ehci-q.c > =================================================================== > --- usb.orig/drivers/usb/host/ehci-q.c > +++ usb/drivers/usb/host/ehci-q.c > @@ -33,12 +33,13 @@ > > /* fill a qtd, returning how much of the buffer we were able to queue up */ > > -static int > +static unsigned int > qtd_fill(struct ehci_hcd *ehci, struct ehci_qtd *qtd, dma_addr_t buf, > size_t len, int token, int maxpacket) > { > - int i, count; > + unsigned int count; > u64 addr = buf; > + int i; > > /* one buffer entry per 4K ... first might be short or unaligned */ > qtd->hw_buf[0] = cpu_to_hc32(ehci, (u32)addr); > @@ -652,7 +653,7 @@ qh_urb_transaction ( > * and may serve as a control status ack > */ > for (;;) { > - int this_qtd_len; > + unsigned int this_qtd_len; > > this_qtd_len = qtd_fill(ehci, qtd, buf, this_sg_len, token, > maxpacket);
Index: usb/drivers/usb/host/ehci-q.c =================================================================== --- usb.orig/drivers/usb/host/ehci-q.c +++ usb/drivers/usb/host/ehci-q.c @@ -33,12 +33,13 @@ /* fill a qtd, returning how much of the buffer we were able to queue up */ -static int +static unsigned int qtd_fill(struct ehci_hcd *ehci, struct ehci_qtd *qtd, dma_addr_t buf, size_t len, int token, int maxpacket) { - int i, count; + unsigned int count; u64 addr = buf; + int i; /* one buffer entry per 4K ... first might be short or unaligned */ qtd->hw_buf[0] = cpu_to_hc32(ehci, (u32)addr); @@ -652,7 +653,7 @@ qh_urb_transaction ( * and may serve as a control status ack */ for (;;) { - int this_qtd_len; + unsigned int this_qtd_len; this_qtd_len = qtd_fill(ehci, qtd, buf, this_sg_len, token, maxpacket);
At the end of qtd_fill(), we assign the 'int count' variable to the 'size_t length' field of 'struct ehci_qtd' -- which implies a problematic type cast. Let's make that variable and the function's result *unsigned int* instead... Found by Linux Verification Center (linuxtesting.org) with the SVACE static analysis tool. Signed-off-by: Sergey Shtylyov <s.shtylyov@omp.ru> --- This patch is against the 'usb-next' branch of Greg KH's 'usb.git' repo. Changes in version 2: - changed 'u16' to *unsigned int* everywhere, repordering the declarations; - updated the type of the 'this_qtd_len' local variable qh_urb_transaction(); - rewrote the patch description. drivers/usb/host/ehci-q.c | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-)