* [PATCH 0/2] Fix uninit and signed integer overflow errors @ 2022-11-03 13:12 Shuah Khan 2022-11-03 13:12 ` [PATCH 1/2] usb/usbip: fix uninitialized variables errors Shuah Khan 2022-11-03 13:12 ` [PATCH 2/2] usb/usbip: Fix v_recv_cmd_submit() to use PIPE_BULK define Shuah Khan 0 siblings, 2 replies; 7+ messages in thread From: Shuah Khan @ 2022-11-03 13:12 UTC (permalink / raw) To: shuah, valentina.manea.m; +Cc: Shuah Khan, gregkh, linux-usb, linux-kernel Fix uninitialized variable and signed integer overflow errors reported by cppcheck. Shuah Khan (2): usb/usbip: fix uninitialized variables errors usb/usbip: Fix v_recv_cmd_submit() to use PIPE_BULK define drivers/usb/usbip/stub_main.c | 2 +- drivers/usb/usbip/stub_rx.c | 4 ++-- drivers/usb/usbip/stub_tx.c | 4 ++-- drivers/usb/usbip/usbip_event.c | 2 +- drivers/usb/usbip/vhci_hcd.c | 2 +- drivers/usb/usbip/vhci_rx.c | 2 +- drivers/usb/usbip/vhci_tx.c | 4 ++-- drivers/usb/usbip/vudc_dev.c | 2 +- drivers/usb/usbip/vudc_rx.c | 7 +++++-- drivers/usb/usbip/vudc_transfer.c | 4 ++-- 10 files changed, 18 insertions(+), 15 deletions(-) -- 2.34.1 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 1/2] usb/usbip: fix uninitialized variables errors 2022-11-03 13:12 [PATCH 0/2] Fix uninit and signed integer overflow errors Shuah Khan @ 2022-11-03 13:12 ` Shuah Khan 2022-11-03 13:21 ` Greg KH 2022-11-03 13:12 ` [PATCH 2/2] usb/usbip: Fix v_recv_cmd_submit() to use PIPE_BULK define Shuah Khan 1 sibling, 1 reply; 7+ messages in thread From: Shuah Khan @ 2022-11-03 13:12 UTC (permalink / raw) To: shuah, valentina.manea.m; +Cc: Shuah Khan, gregkh, linux-usb, linux-kernel Fix uninitialized variable errors reported by cppcheck. One example below. usbip/stub_main.c:284:10: error: Uninitialized variables: priv.seqnum, priv.sdev, priv.urbs, priv.sgl, priv.num_urbs, priv.completed_urbs, priv.urb_status, priv.unlinking [uninitvar] return priv; Signed-off-by: Shuah Khan <skhan@linuxfoundation.org> --- drivers/usb/usbip/stub_main.c | 2 +- drivers/usb/usbip/stub_rx.c | 4 ++-- drivers/usb/usbip/stub_tx.c | 4 ++-- drivers/usb/usbip/usbip_event.c | 2 +- drivers/usb/usbip/vhci_hcd.c | 2 +- drivers/usb/usbip/vhci_rx.c | 2 +- drivers/usb/usbip/vhci_tx.c | 4 ++-- drivers/usb/usbip/vudc_dev.c | 2 +- drivers/usb/usbip/vudc_rx.c | 2 +- drivers/usb/usbip/vudc_transfer.c | 4 ++-- 10 files changed, 14 insertions(+), 14 deletions(-) diff --git a/drivers/usb/usbip/stub_main.c b/drivers/usb/usbip/stub_main.c index e8c3131a8543..e1248b971218 100644 --- a/drivers/usb/usbip/stub_main.c +++ b/drivers/usb/usbip/stub_main.c @@ -277,7 +277,7 @@ static DRIVER_ATTR_WO(rebind); static struct stub_priv *stub_priv_pop_from_listhead(struct list_head *listhead) { - struct stub_priv *priv, *tmp; + struct stub_priv *priv = NULL, *tmp; list_for_each_entry_safe(priv, tmp, listhead, list) { list_del_init(&priv->list); diff --git a/drivers/usb/usbip/stub_rx.c b/drivers/usb/usbip/stub_rx.c index fc01b31bbb87..08e11948834b 100644 --- a/drivers/usb/usbip/stub_rx.c +++ b/drivers/usb/usbip/stub_rx.c @@ -206,7 +206,7 @@ static int stub_recv_cmd_unlink(struct stub_device *sdev, { int ret, i; unsigned long flags; - struct stub_priv *priv; + struct stub_priv *priv = NULL; spin_lock_irqsave(&sdev->priv_lock, flags); @@ -440,7 +440,7 @@ static void masking_bogus_flags(struct urb *urb) static int stub_recv_xbuff(struct usbip_device *ud, struct stub_priv *priv) { - int ret; + int ret = 0; int i; for (i = 0; i < priv->num_urbs; i++) { diff --git a/drivers/usb/usbip/stub_tx.c b/drivers/usb/usbip/stub_tx.c index b1c2f6781cb3..3d3d2b5a680d 100644 --- a/drivers/usb/usbip/stub_tx.c +++ b/drivers/usb/usbip/stub_tx.c @@ -132,7 +132,7 @@ static void setup_ret_unlink_pdu(struct usbip_header *rpdu, static struct stub_priv *dequeue_from_priv_tx(struct stub_device *sdev) { unsigned long flags; - struct stub_priv *priv, *tmp; + struct stub_priv *priv = NULL, *tmp; spin_lock_irqsave(&sdev->priv_lock, flags); @@ -343,7 +343,7 @@ static int stub_send_ret_submit(struct stub_device *sdev) static struct stub_unlink *dequeue_from_unlink_tx(struct stub_device *sdev) { unsigned long flags; - struct stub_unlink *unlink, *tmp; + struct stub_unlink *unlink = NULL, *tmp; spin_lock_irqsave(&sdev->priv_lock, flags); diff --git a/drivers/usb/usbip/usbip_event.c b/drivers/usb/usbip/usbip_event.c index 26513540bcdb..8d782c8d8893 100644 --- a/drivers/usb/usbip/usbip_event.c +++ b/drivers/usb/usbip/usbip_event.c @@ -143,7 +143,7 @@ void usbip_finish_eh(void) void usbip_event_add(struct usbip_device *ud, unsigned long event) { - struct usbip_event *ue; + struct usbip_event *ue = NULL; unsigned long flags; if (ud->event & USBIP_EH_BYE) diff --git a/drivers/usb/usbip/vhci_hcd.c b/drivers/usb/usbip/vhci_hcd.c index 233265550fc6..79503ffc3db8 100644 --- a/drivers/usb/usbip/vhci_hcd.c +++ b/drivers/usb/usbip/vhci_hcd.c @@ -957,7 +957,7 @@ static void vhci_cleanup_unlink_list(struct vhci_device *vdev, struct vhci_hcd *vhci_hcd = vdev_to_vhci_hcd(vdev); struct usb_hcd *hcd = vhci_hcd_to_hcd(vhci_hcd); struct vhci *vhci = vhci_hcd->vhci; - struct vhci_unlink *unlink, *tmp; + struct vhci_unlink *unlink = NULL, *tmp; unsigned long flags; spin_lock_irqsave(&vhci->lock, flags); diff --git a/drivers/usb/usbip/vhci_rx.c b/drivers/usb/usbip/vhci_rx.c index 7f2d1c241559..8edb699aed23 100644 --- a/drivers/usb/usbip/vhci_rx.c +++ b/drivers/usb/usbip/vhci_rx.c @@ -12,7 +12,7 @@ /* get URB from transmitted urb queue. caller must hold vdev->priv_lock */ struct urb *pickup_urb_and_free_priv(struct vhci_device *vdev, __u32 seqnum) { - struct vhci_priv *priv, *tmp; + struct vhci_priv *priv = NULL, *tmp; struct urb *urb = NULL; int status; diff --git a/drivers/usb/usbip/vhci_tx.c b/drivers/usb/usbip/vhci_tx.c index 0ae40a13a9fe..78b96c915724 100644 --- a/drivers/usb/usbip/vhci_tx.c +++ b/drivers/usb/usbip/vhci_tx.c @@ -33,7 +33,7 @@ static void setup_cmd_submit_pdu(struct usbip_header *pdup, struct urb *urb) static struct vhci_priv *dequeue_from_priv_tx(struct vhci_device *vdev) { - struct vhci_priv *priv, *tmp; + struct vhci_priv *priv = NULL, *tmp; unsigned long flags; spin_lock_irqsave(&vdev->priv_lock, flags); @@ -168,7 +168,7 @@ static int vhci_send_cmd_submit(struct vhci_device *vdev) static struct vhci_unlink *dequeue_from_unlink_tx(struct vhci_device *vdev) { - struct vhci_unlink *unlink, *tmp; + struct vhci_unlink *unlink = NULL, *tmp; unsigned long flags; spin_lock_irqsave(&vdev->priv_lock, flags); diff --git a/drivers/usb/usbip/vudc_dev.c b/drivers/usb/usbip/vudc_dev.c index 2bc428f2e261..ddf7e6f18439 100644 --- a/drivers/usb/usbip/vudc_dev.c +++ b/drivers/usb/usbip/vudc_dev.c @@ -90,7 +90,7 @@ static void nuke(struct vudc *udc, struct vep *ep) static void stop_activity(struct vudc *udc) { int i; - struct urbp *urb_p, *tmp; + struct urbp *urb_p = NULL, *tmp; udc->address = 0; diff --git a/drivers/usb/usbip/vudc_rx.c b/drivers/usb/usbip/vudc_rx.c index d4a2f30a7580..a6ca27f10b68 100644 --- a/drivers/usb/usbip/vudc_rx.c +++ b/drivers/usb/usbip/vudc_rx.c @@ -63,7 +63,7 @@ static int v_recv_cmd_unlink(struct vudc *udc, struct usbip_header *pdu) { unsigned long flags; - struct urbp *urb_p; + struct urbp *urb_p = NULL; spin_lock_irqsave(&udc->lock, flags); list_for_each_entry(urb_p, &udc->urb_queue, urb_entry) { diff --git a/drivers/usb/usbip/vudc_transfer.c b/drivers/usb/usbip/vudc_transfer.c index 7e801fee33bf..fd5547f85de9 100644 --- a/drivers/usb/usbip/vudc_transfer.c +++ b/drivers/usb/usbip/vudc_transfer.c @@ -183,7 +183,7 @@ static int handle_control_request(struct vudc *udc, struct urb *urb, static int transfer(struct vudc *udc, struct urb *urb, struct vep *ep, int limit) { - struct vrequest *req; + struct vrequest *req = NULL; int sent = 0; top: /* if there's no request queued, the device is NAKing; return */ @@ -303,7 +303,7 @@ static void v_timer(struct timer_list *t) { struct vudc *udc = from_timer(udc, t, tr_timer.timer); struct transfer_timer *timer = &udc->tr_timer; - struct urbp *urb_p, *tmp; + struct urbp *urb_p = NULL, *tmp; unsigned long flags; struct usb_ep *_ep; struct vep *ep; -- 2.34.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] usb/usbip: fix uninitialized variables errors 2022-11-03 13:12 ` [PATCH 1/2] usb/usbip: fix uninitialized variables errors Shuah Khan @ 2022-11-03 13:21 ` Greg KH 2022-11-04 15:18 ` Shuah Khan 0 siblings, 1 reply; 7+ messages in thread From: Greg KH @ 2022-11-03 13:21 UTC (permalink / raw) To: Shuah Khan; +Cc: shuah, valentina.manea.m, linux-usb, linux-kernel On Thu, Nov 03, 2022 at 07:12:42AM -0600, Shuah Khan wrote: > Fix uninitialized variable errors reported by cppcheck. One example > below. > > usbip/stub_main.c:284:10: error: Uninitialized variables: priv.seqnum, priv.sdev, priv.urbs, priv.sgl, priv.num_urbs, priv.completed_urbs, priv.urb_status, priv.unlinking [uninitvar] > return priv; > > Signed-off-by: Shuah Khan <skhan@linuxfoundation.org> > --- > drivers/usb/usbip/stub_main.c | 2 +- > drivers/usb/usbip/stub_rx.c | 4 ++-- > drivers/usb/usbip/stub_tx.c | 4 ++-- > drivers/usb/usbip/usbip_event.c | 2 +- > drivers/usb/usbip/vhci_hcd.c | 2 +- > drivers/usb/usbip/vhci_rx.c | 2 +- > drivers/usb/usbip/vhci_tx.c | 4 ++-- > drivers/usb/usbip/vudc_dev.c | 2 +- > drivers/usb/usbip/vudc_rx.c | 2 +- > drivers/usb/usbip/vudc_transfer.c | 4 ++-- > 10 files changed, 14 insertions(+), 14 deletions(-) > > diff --git a/drivers/usb/usbip/stub_main.c b/drivers/usb/usbip/stub_main.c > index e8c3131a8543..e1248b971218 100644 > --- a/drivers/usb/usbip/stub_main.c > +++ b/drivers/usb/usbip/stub_main.c > @@ -277,7 +277,7 @@ static DRIVER_ATTR_WO(rebind); > > static struct stub_priv *stub_priv_pop_from_listhead(struct list_head *listhead) > { > - struct stub_priv *priv, *tmp; > + struct stub_priv *priv = NULL, *tmp; > > list_for_each_entry_safe(priv, tmp, listhead, list) { cppcheck is wrong here, the code is fine, and setting priv to NULL does nothing. If it was required, gcc would have hopefully caught it, and the code would have never worked :) So are you sure all of these changes are really needed? Last time I looked, cppcheck wasn't all that smart when it came to the kernel and threw up huge numbers of false-positives, like this one. thanks, greg k-h ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 1/2] usb/usbip: fix uninitialized variables errors 2022-11-03 13:21 ` Greg KH @ 2022-11-04 15:18 ` Shuah Khan 0 siblings, 0 replies; 7+ messages in thread From: Shuah Khan @ 2022-11-04 15:18 UTC (permalink / raw) To: Greg KH; +Cc: shuah, valentina.manea.m, linux-usb, linux-kernel, Shuah Khan On 11/3/22 07:21, Greg KH wrote: > On Thu, Nov 03, 2022 at 07:12:42AM -0600, Shuah Khan wrote: >> Fix uninitialized variable errors reported by cppcheck. One example >> below. >> >> usbip/stub_main.c:284:10: error: Uninitialized variables: priv.seqnum, priv.sdev, priv.urbs, priv.sgl, priv.num_urbs, priv.completed_urbs, priv.urb_status, priv.unlinking [uninitvar] >> return priv; >> >> Signed-off-by: Shuah Khan <skhan@linuxfoundation.org> >> --- >> drivers/usb/usbip/stub_main.c | 2 +- >> drivers/usb/usbip/stub_rx.c | 4 ++-- >> drivers/usb/usbip/stub_tx.c | 4 ++-- >> drivers/usb/usbip/usbip_event.c | 2 +- >> drivers/usb/usbip/vhci_hcd.c | 2 +- >> drivers/usb/usbip/vhci_rx.c | 2 +- >> drivers/usb/usbip/vhci_tx.c | 4 ++-- >> drivers/usb/usbip/vudc_dev.c | 2 +- >> drivers/usb/usbip/vudc_rx.c | 2 +- >> drivers/usb/usbip/vudc_transfer.c | 4 ++-- >> 10 files changed, 14 insertions(+), 14 deletions(-) >> >> diff --git a/drivers/usb/usbip/stub_main.c b/drivers/usb/usbip/stub_main.c >> index e8c3131a8543..e1248b971218 100644 >> --- a/drivers/usb/usbip/stub_main.c >> +++ b/drivers/usb/usbip/stub_main.c >> @@ -277,7 +277,7 @@ static DRIVER_ATTR_WO(rebind); >> >> static struct stub_priv *stub_priv_pop_from_listhead(struct list_head *listhead) >> { >> - struct stub_priv *priv, *tmp; >> + struct stub_priv *priv = NULL, *tmp; >> >> list_for_each_entry_safe(priv, tmp, listhead, list) { > > cppcheck is wrong here, the code is fine, and setting priv to NULL does > nothing. If it was required, gcc would have hopefully caught it, and > the code would have never worked :) > > So are you sure all of these changes are really needed? Last time I > looked, cppcheck wasn't all that smart when it came to the kernel and > threw up huge numbers of false-positives, like this one. > You are right that this one is a false positive. I will take a close look at the others in this patch. thanks, -- Shuah ^ permalink raw reply [flat|nested] 7+ messages in thread
* [PATCH 2/2] usb/usbip: Fix v_recv_cmd_submit() to use PIPE_BULK define 2022-11-03 13:12 [PATCH 0/2] Fix uninit and signed integer overflow errors Shuah Khan 2022-11-03 13:12 ` [PATCH 1/2] usb/usbip: fix uninitialized variables errors Shuah Khan @ 2022-11-03 13:12 ` Shuah Khan 2022-11-03 13:24 ` Greg KH 1 sibling, 1 reply; 7+ messages in thread From: Shuah Khan @ 2022-11-03 13:12 UTC (permalink / raw) To: shuah, valentina.manea.m; +Cc: Shuah Khan, gregkh, linux-usb, linux-kernel 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 + urb_p->urb->pipe &= ~(PIPE_BULK << 30); switch (urb_p->ep->type) { case USB_ENDPOINT_XFER_BULK: urb_p->urb->pipe |= (PIPE_BULK << 30); -- 2.34.1 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] usb/usbip: Fix v_recv_cmd_submit() to use PIPE_BULK define 2022-11-03 13:12 ` [PATCH 2/2] usb/usbip: Fix v_recv_cmd_submit() to use PIPE_BULK define Shuah Khan @ 2022-11-03 13:24 ` Greg KH 2022-11-04 15:19 ` Shuah Khan 0 siblings, 1 reply; 7+ messages in thread From: Greg KH @ 2022-11-03 13:24 UTC (permalink / raw) To: Shuah Khan; +Cc: shuah, valentina.manea.m, linux-usb, linux-kernel 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH 2/2] usb/usbip: Fix v_recv_cmd_submit() to use PIPE_BULK define 2022-11-03 13:24 ` Greg KH @ 2022-11-04 15:19 ` Shuah Khan 0 siblings, 0 replies; 7+ messages in thread From: Shuah Khan @ 2022-11-04 15:19 UTC (permalink / raw) To: Greg KH; +Cc: shuah, valentina.manea.m, linux-usb, linux-kernel, Shuah Khan 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2022-11-04 15:19 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2022-11-03 13:12 [PATCH 0/2] Fix uninit and signed integer overflow errors Shuah Khan 2022-11-03 13:12 ` [PATCH 1/2] usb/usbip: fix uninitialized variables errors Shuah Khan 2022-11-03 13:21 ` Greg KH 2022-11-04 15:18 ` Shuah Khan 2022-11-03 13:12 ` [PATCH 2/2] usb/usbip: Fix v_recv_cmd_submit() to use PIPE_BULK define Shuah Khan 2022-11-03 13:24 ` Greg KH 2022-11-04 15:19 ` Shuah Khan
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).