* staging: emxx_udc question on i_write_length datatype
@ 2022-11-03 7:27 Deepak R Varma
2022-11-03 7:55 ` Dan Carpenter
0 siblings, 1 reply; 3+ messages in thread
From: Deepak R Varma @ 2022-11-03 7:27 UTC (permalink / raw)
To: outreachy, Greg Kroah-Hartman, linux-staging, linux-kernel
Hello,
While reviewing this [1] coccicheck warning, I observed something that concerned
me. The variable i_write_length is declared to be of u32 type. Later it is
assigned a value DMA_MAX_COUNT * mpkt; which is 256 * u32;
I am unable to estimate if mpkt (or max packet size) can attain value greater
than 16777215 in which case the result will overflow the 32 bits of
i_write_length. Is it safe to make i_write_length to be a u64?
[1] drivers/staging/emxx_udc/emxx_udc.c:1007:28-29: WARNING opportunity for min()
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: staging: emxx_udc question on i_write_length datatype
2022-11-03 7:27 staging: emxx_udc question on i_write_length datatype Deepak R Varma
@ 2022-11-03 7:55 ` Dan Carpenter
2022-11-03 8:12 ` Deepak R Varma
0 siblings, 1 reply; 3+ messages in thread
From: Dan Carpenter @ 2022-11-03 7:55 UTC (permalink / raw)
To: Deepak R Varma; +Cc: outreachy, Greg Kroah-Hartman, linux-staging, linux-kernel
On Thu, Nov 03, 2022 at 12:57:09PM +0530, Deepak R Varma wrote:
> Hello,
> While reviewing this [1] coccicheck warning, I observed something that concerned
> me. The variable i_write_length is declared to be of u32 type. Later it is
> assigned a value DMA_MAX_COUNT * mpkt; which is 256 * u32;
>
> I am unable to estimate if mpkt (or max packet size) can attain value greater
> than 16777215 in which case the result will overflow the 32 bits of
> i_write_length. Is it safe to make i_write_length to be a u64?
>
> [1] drivers/staging/emxx_udc/emxx_udc.c:1007:28-29: WARNING opportunity for min()
>
drivers/staging/emxx_udc/emxx_udc.c
983 static int _nbu2ss_in_dma(struct nbu2ss_udc *udc, struct nbu2ss_ep *ep,
984 struct nbu2ss_req *req, u32 num, u32 length)
985 {
986 dma_addr_t p_buffer;
987 u32 mpkt; /* MaxPacketSize */
988 u32 lmpkt; /* Last Packet Data Size */
989 u32 dmacnt; /* IN Data Size */
990 u32 i_write_length;
991 u32 data;
992 int result = -EINVAL;
993 struct fc_regs __iomem *preg = udc->p_regs;
994
995 if (req->dma_flag)
996 return 1; /* DMA is forwarded */
997
998 #ifdef USE_DMA
999 if (req->req.actual == 0)
1000 _nbu2ss_dma_map_single(udc, ep, req, USB_DIR_IN);
1001 #endif
1002 req->dma_flag = true;
1003
1004 /* MAX Packet Size */
1005 mpkt = _nbu2ss_readl(&preg->EP_REGS[num].EP_PCKT_ADRS) & EPN_MPKT;
^^^^^^^^
mpkt is 0-0x7ff so 256 * 0x7ff will not be greater than UINT_MAX.
1006
1007 if ((DMA_MAX_COUNT * mpkt) < length)
1008 i_write_length = DMA_MAX_COUNT * mpkt;
1009 else
1010 i_write_length = length;
1011
1012 /*------------------------------------------------------------*/
1013 /* Number of transmission packets */
1014 if (mpkt < i_write_length) {
regards,
dan carpenter
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: staging: emxx_udc question on i_write_length datatype
2022-11-03 7:55 ` Dan Carpenter
@ 2022-11-03 8:12 ` Deepak R Varma
0 siblings, 0 replies; 3+ messages in thread
From: Deepak R Varma @ 2022-11-03 8:12 UTC (permalink / raw)
To: Dan Carpenter; +Cc: outreachy, Greg Kroah-Hartman, linux-staging, linux-kernel
On Thu, Nov 03, 2022 at 10:55:27AM +0300, Dan Carpenter wrote:
> On Thu, Nov 03, 2022 at 12:57:09PM +0530, Deepak R Varma wrote:
> > Hello,
> > While reviewing this [1] coccicheck warning, I observed something that concerned
> > me. The variable i_write_length is declared to be of u32 type. Later it is
> > assigned a value DMA_MAX_COUNT * mpkt; which is 256 * u32;
> >
> > I am unable to estimate if mpkt (or max packet size) can attain value greater
> > than 16777215 in which case the result will overflow the 32 bits of
> > i_write_length. Is it safe to make i_write_length to be a u64?
> >
> > [1] drivers/staging/emxx_udc/emxx_udc.c:1007:28-29: WARNING opportunity for min()
> >
>
> drivers/staging/emxx_udc/emxx_udc.c
> 983 static int _nbu2ss_in_dma(struct nbu2ss_udc *udc, struct nbu2ss_ep *ep,
> 984 struct nbu2ss_req *req, u32 num, u32 length)
> 985 {
> 986 dma_addr_t p_buffer;
> 987 u32 mpkt; /* MaxPacketSize */
> 988 u32 lmpkt; /* Last Packet Data Size */
> 989 u32 dmacnt; /* IN Data Size */
> 990 u32 i_write_length;
> 991 u32 data;
> 992 int result = -EINVAL;
> 993 struct fc_regs __iomem *preg = udc->p_regs;
> 994
> 995 if (req->dma_flag)
> 996 return 1; /* DMA is forwarded */
> 997
> 998 #ifdef USE_DMA
> 999 if (req->req.actual == 0)
> 1000 _nbu2ss_dma_map_single(udc, ep, req, USB_DIR_IN);
> 1001 #endif
> 1002 req->dma_flag = true;
> 1003
> 1004 /* MAX Packet Size */
> 1005 mpkt = _nbu2ss_readl(&preg->EP_REGS[num].EP_PCKT_ADRS) & EPN_MPKT;
> ^^^^^^^^
> mpkt is 0-0x7ff so 256 * 0x7ff will not be greater than UINT_MAX.
Thank you Dan. I understand now. "& EPN_MPKT" keeps the size under control.
Appreciate very much.
./drv
>
> 1006
> 1007 if ((DMA_MAX_COUNT * mpkt) < length)
> 1008 i_write_length = DMA_MAX_COUNT * mpkt;
> 1009 else
> 1010 i_write_length = length;
> 1011
> 1012 /*------------------------------------------------------------*/
> 1013 /* Number of transmission packets */
> 1014 if (mpkt < i_write_length) {
>
> regards,
> dan carpenter
>
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2022-11-03 8:13 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2022-11-03 7:27 staging: emxx_udc question on i_write_length datatype Deepak R Varma
2022-11-03 7:55 ` Dan Carpenter
2022-11-03 8:12 ` Deepak R Varma
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox