diff mbox series

[2/2] usb/usbip: Fix v_recv_cmd_submit() to use PIPE_BULK define

Message ID c9790c485bfe31c55bbd2f9b270548ecefddc91a.1667480280.git.skhan@linuxfoundation.org (mailing list archive)
State New, archived
Headers show
Series Fix uninit and signed integer overflow errors | expand

Commit Message

Shuah Khan Nov. 3, 2022, 1:12 p.m. UTC
Fix v_recv_cmd_submit() to use PIPE_BULK define instead of hard coded
values. This also fixes the following signed integer overflow error
reported by cppcheck. This is not an issue since pipe is unsigned int.
However, this change improves the code to use proper define.

drivers/usb/usbip/vudc_rx.c:152:26: error: Signed integer overflow for expression '3<<30'. [integerOverflow]
 urb_p->urb->pipe &= ~(3 << 30);

In addition, add a sanity check for PIPE_BULK != 3 as the code path depends
on PIPE_BULK = 3.

Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
---
 drivers/usb/usbip/vudc_rx.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

Comments

Greg Kroah-Hartman Nov. 3, 2022, 1:24 p.m. UTC | #1
On Thu, Nov 03, 2022 at 07:12:43AM -0600, Shuah Khan wrote:
> Fix v_recv_cmd_submit() to use PIPE_BULK define instead of hard coded
> values. This also fixes the following signed integer overflow error
> reported by cppcheck. This is not an issue since pipe is unsigned int.
> However, this change improves the code to use proper define.
> 
> drivers/usb/usbip/vudc_rx.c:152:26: error: Signed integer overflow for expression '3<<30'. [integerOverflow]
>  urb_p->urb->pipe &= ~(3 << 30);
> 
> In addition, add a sanity check for PIPE_BULK != 3 as the code path depends
> on PIPE_BULK = 3.
> 
> Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
> ---
>  drivers/usb/usbip/vudc_rx.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/usb/usbip/vudc_rx.c b/drivers/usb/usbip/vudc_rx.c
> index a6ca27f10b68..e7e0eb6bbca0 100644
> --- a/drivers/usb/usbip/vudc_rx.c
> +++ b/drivers/usb/usbip/vudc_rx.c
> @@ -149,7 +149,10 @@ static int v_recv_cmd_submit(struct vudc *udc,
>  	urb_p->urb->status = -EINPROGRESS;
>  
>  	/* FIXME: more pipe setup to please usbip_common */
> -	urb_p->urb->pipe &= ~(3 << 30);
> +#if PIPE_BULK != 3
> +#error "Sanity check failed, this code presumes PIPE_... to range from 0 to 3"
> +#endif

Perhaps use BUILD_BUG_ON() instead of hand-rolling one?

thanks,

greg k-h
Shuah Khan Nov. 4, 2022, 3:19 p.m. UTC | #2
On 11/3/22 07:24, Greg KH wrote:
> On Thu, Nov 03, 2022 at 07:12:43AM -0600, Shuah Khan wrote:
>> Fix v_recv_cmd_submit() to use PIPE_BULK define instead of hard coded
>> values. This also fixes the following signed integer overflow error
>> reported by cppcheck. This is not an issue since pipe is unsigned int.
>> However, this change improves the code to use proper define.
>>
>> drivers/usb/usbip/vudc_rx.c:152:26: error: Signed integer overflow for expression '3<<30'. [integerOverflow]
>>   urb_p->urb->pipe &= ~(3 << 30);
>>
>> In addition, add a sanity check for PIPE_BULK != 3 as the code path depends
>> on PIPE_BULK = 3.
>>
>> Signed-off-by: Shuah Khan <skhan@linuxfoundation.org>
>> ---
>>   drivers/usb/usbip/vudc_rx.c | 5 ++++-
>>   1 file changed, 4 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/usb/usbip/vudc_rx.c b/drivers/usb/usbip/vudc_rx.c
>> index a6ca27f10b68..e7e0eb6bbca0 100644
>> --- a/drivers/usb/usbip/vudc_rx.c
>> +++ b/drivers/usb/usbip/vudc_rx.c
>> @@ -149,7 +149,10 @@ static int v_recv_cmd_submit(struct vudc *udc,
>>   	urb_p->urb->status = -EINPROGRESS;
>>   
>>   	/* FIXME: more pipe setup to please usbip_common */
>> -	urb_p->urb->pipe &= ~(3 << 30);
>> +#if PIPE_BULK != 3
>> +#error "Sanity check failed, this code presumes PIPE_... to range from 0 to 3"
>> +#endif
> 
> Perhaps use BUILD_BUG_ON() instead of hand-rolling one?
> 

Makes sense. I will send v2.

thanks,
-- Shuah
diff mbox series

Patch

diff --git a/drivers/usb/usbip/vudc_rx.c b/drivers/usb/usbip/vudc_rx.c
index a6ca27f10b68..e7e0eb6bbca0 100644
--- a/drivers/usb/usbip/vudc_rx.c
+++ b/drivers/usb/usbip/vudc_rx.c
@@ -149,7 +149,10 @@  static int v_recv_cmd_submit(struct vudc *udc,
 	urb_p->urb->status = -EINPROGRESS;
 
 	/* FIXME: more pipe setup to please usbip_common */
-	urb_p->urb->pipe &= ~(3 << 30);
+#if PIPE_BULK != 3
+#error "Sanity check failed, this code presumes PIPE_... to range from 0 to 3"
+#endif
+	urb_p->urb->pipe &= ~(PIPE_BULK << 30);
 	switch (urb_p->ep->type) {
 	case USB_ENDPOINT_XFER_BULK:
 		urb_p->urb->pipe |= (PIPE_BULK << 30);