netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* re: usbnet: fix oops in usbnet_start_xmit
@ 2011-11-09  7:34 Dan Carpenter
       [not found] ` <20111109073459.GA14409-mgFCXtclrQlZLf2FXnZxJA@public.gmane.org>
  0 siblings, 1 reply; 3+ messages in thread
From: Dan Carpenter @ 2011-11-09  7:34 UTC (permalink / raw)
  To: khlebnikov-GEFAQzZX7r8dnm+yROfE0A
  Cc: netdev-u79uwXL29TY76Z2rM5mHXA, linux-usb-u79uwXL29TY76Z2rM5mHXA

Hello Konstantin Khlebnikov,

This is a semi-automatic email about new static checker warnings.

The patch 23ba07991dad: "usbnet: fix oops in usbnet_start_xmit" from 
Nov 7, 2011, leads to the following Smatch complaint:

drivers/net/usb/usbnet.c +1077 usbnet_start_xmit()
	 error: we previously assumed 'skb' could be null (see line 1060)

drivers/net/usb/usbnet.c
  1059	
  1060		if (skb)
                    ^^^
check introduced here.

  1061			skb_tx_timestamp(skb);
  1062	
  1063		// some devices want funky USB-level framing, for
  1064		// win32 driver (usually) and/or hardware quirks
  1065		if (info->tx_fixup) {
  1066			skb = info->tx_fixup (dev, skb, GFP_ATOMIC);
  1067			if (!skb) {
  1068				if (netif_msg_tx_err(dev)) {
  1069					netif_dbg(dev, tx_err, dev->net, "can't tx_fixup skb\n");
  1070					goto drop;
  1071				} else {
  1072					/* cdc_ncm collected packet; waits for more */
  1073					goto not_drop;
  1074				}
  1075			}
  1076		}
  1077		length = skb->len;
                         ^^^^^^^^
dereference without checking.

  1078	
  1079		if (!(urb = usb_alloc_urb (0, GFP_ATOMIC))) {

regards,
dan carpenter

--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: usbnet: fix oops in usbnet_start_xmit
       [not found] ` <20111109073459.GA14409-mgFCXtclrQlZLf2FXnZxJA@public.gmane.org>
@ 2012-09-06 12:52   ` Dan Carpenter
  2012-09-06 13:10     ` Oliver Neukum
  0 siblings, 1 reply; 3+ messages in thread
From: Dan Carpenter @ 2012-09-06 12:52 UTC (permalink / raw)
  To: khlebnikov-GEFAQzZX7r8dnm+yROfE0A
  Cc: netdev-u79uwXL29TY76Z2rM5mHXA, linux-usb-u79uwXL29TY76Z2rM5mHXA

I sent this email a year ago when the patch was committed but I
never received a response.

regards,
dan carpenter

On Wed, Nov 09, 2011 at 10:34:59AM +0300, Dan Carpenter wrote:
> Hello Konstantin Khlebnikov,
> 
> This is a semi-automatic email about new static checker warnings.
> 
> The patch 23ba07991dad: "usbnet: fix oops in usbnet_start_xmit" from 
> Nov 7, 2011, leads to the following Smatch complaint:
> 
> drivers/net/usb/usbnet.c +1077 usbnet_start_xmit()
> 	 error: we previously assumed 'skb' could be null (see line 1060)
> 
> drivers/net/usb/usbnet.c
>   1059	
>   1060		if (skb)
>                     ^^^
> check introduced here.
> 
>   1061			skb_tx_timestamp(skb);
>   1062	
>   1063		// some devices want funky USB-level framing, for
>   1064		// win32 driver (usually) and/or hardware quirks
>   1065		if (info->tx_fixup) {
>   1066			skb = info->tx_fixup (dev, skb, GFP_ATOMIC);
>   1067			if (!skb) {
>   1068				if (netif_msg_tx_err(dev)) {
>   1069					netif_dbg(dev, tx_err, dev->net, "can't tx_fixup skb\n");
>   1070					goto drop;
>   1071				} else {
>   1072					/* cdc_ncm collected packet; waits for more */
>   1073					goto not_drop;
>   1074				}
>   1075			}
>   1076		}
>   1077		length = skb->len;
>                          ^^^^^^^^
> dereference without checking.
> 
>   1078	
>   1079		if (!(urb = usb_alloc_urb (0, GFP_ATOMIC))) {
> 
> regards,
> dan carpenter
> 
--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 3+ messages in thread

* Re: usbnet: fix oops in usbnet_start_xmit
  2012-09-06 12:52   ` Dan Carpenter
@ 2012-09-06 13:10     ` Oliver Neukum
  0 siblings, 0 replies; 3+ messages in thread
From: Oliver Neukum @ 2012-09-06 13:10 UTC (permalink / raw)
  To: Dan Carpenter
  Cc: khlebnikov-GEFAQzZX7r8dnm+yROfE0A, netdev-u79uwXL29TY76Z2rM5mHXA,
	linux-usb-u79uwXL29TY76Z2rM5mHXA

On Thursday 06 September 2012 05:52:30 Dan Carpenter wrote:
> I sent this email a year ago when the patch was committed but I
> never received a response.

I probably should have checked then.

> regards,
> dan carpenter
> 
> On Wed, Nov 09, 2011 at 10:34:59AM +0300, Dan Carpenter wrote:
> > Hello Konstantin Khlebnikov,
> > 
> > This is a semi-automatic email about new static checker warnings.
> > 
> > The patch 23ba07991dad: "usbnet: fix oops in usbnet_start_xmit" from 
> > Nov 7, 2011, leads to the following Smatch complaint:
> > 
> > drivers/net/usb/usbnet.c +1077 usbnet_start_xmit()
> >        error: we previously assumed 'skb' could be null (see line 1060)
> > 
> > drivers/net/usb/usbnet.c
> >   1059        
> >   1060                if (skb)
> >                     ^^^
> > check introduced here.
> > 
> >   1061                        skb_tx_timestamp(skb);
> >   1062        
> >   1063                // some devices want funky USB-level framing, for
> >   1064                // win32 driver (usually) and/or hardware quirks
> >   1065                if (info->tx_fixup) {
> >   1066                        skb = info->tx_fixup (dev, skb, GFP_ATOMIC);

It turns out that skb == NULL implies info->tx_fixup != NULL
and skb will be reassigned.
This is very dirty.

	Regards
		Oliver


--
To unsubscribe from this list: send the line "unsubscribe linux-usb" in
the body of a message to majordomo-u79uwXL29TY76Z2rM5mHXA@public.gmane.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

^ permalink raw reply	[flat|nested] 3+ messages in thread

end of thread, other threads:[~2012-09-06 13:10 UTC | newest]

Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-11-09  7:34 usbnet: fix oops in usbnet_start_xmit Dan Carpenter
     [not found] ` <20111109073459.GA14409-mgFCXtclrQlZLf2FXnZxJA@public.gmane.org>
2012-09-06 12:52   ` Dan Carpenter
2012-09-06 13:10     ` Oliver Neukum

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).