re: Microchip's LAN7800 family USB 2/3 to 10/100/1000 Ethernet device driver

re: Microchip's LAN7800 family USB 2/3 to 10/100/1000 Ethernet device driver

[Dan Carpenter]

Hello Woojung.Huh@microchip.com,

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

The patch 55d7de9de6c3: "Microchip's LAN7800 family USB 2/3 to 
10/100/1000 Ethernet device driver" from Jul 30, 2015, leads to the 
following Smatch complaint:

drivers/net/usb/lan78xx.c:2282 tx_complete()
	 warn: variable dereferenced before check 'skb' (see line 2249)

drivers/net/usb/lan78xx.c
  2248		struct sk_buff *skb = (struct sk_buff *)urb->context;
  2249		struct skb_data *entry = (struct skb_data *)skb->cb;
                                                            ^^^^^^^
Dereference.

  2250		struct lan78xx_net *dev = entry->dev;
  2251	
  2252		if (urb->status = 0) {
  2253			dev->net->stats.tx_packets++;
  2254			dev->net->stats.tx_bytes += entry->length;
  2255		} else {
  2256			dev->net->stats.tx_errors++;
  2257	
  2258			switch (urb->status) {
  2259			case -EPIPE:
  2260				lan78xx_defer_kevent(dev, EVENT_TX_HALT);
  2261				break;
  2262	
  2263			/* software-driven interface shutdown */
  2264			case -ECONNRESET:
  2265			case -ESHUTDOWN:
  2266				break;
  2267	
  2268			case -EPROTO:
  2269			case -ETIME:
  2270			case -EILSEQ:
  2271				netif_stop_queue(dev->net);
  2272				break;
  2273			default:
  2274				netif_dbg(dev, tx_err, dev->net,
  2275					  "tx err %d\n", entry->urb->status);
  2276				break;
  2277			}
  2278		}
  2279	
  2280		usb_autopm_put_interface_async(dev->intf);
  2281	
  2282		if (skb)
                    ^^^
Check too late.

  2283			defer_bh(dev, skb, &dev->txq, tx_done);
  2284	}

regards,
dan carpenter

Re: Microchip's LAN7800 family USB 2/3 to 10/100/1000 Ethernet device driver

[Dan Carpenter]

There are some other Smatch warnings as well:

drivers/net/usb/lan78xx.c:2302 lan78xx_start_xmit() error: we previously assumed 'skb' could be null (see line 2299)
skb is dereferenced inside the function call.

drivers/net/usb/lan78xx.c:2885 lan78xx_bh() info: ignoring unreachable code.
drivers/net/usb/lan78xx.c:3159 lan78xx_probe() info: ignoring unreachable code.

These are calls to BUG_ON().  You should just delete it, network
drivers shouldn't call BUG_ON(), at most they should call WARN_ON().

regards,
dan carpenter

RE: Microchip's LAN7800 family USB 2/3 to 10/100/1000 Ethernet device driver

[Woojung.Huh]

Hi Dan,

Thanks for comments.
I'll look into it and get back to you.

Where do I post patches?

Thanks,
Woojung

> -----Original Message-----
> From: Dan Carpenter [mailto:dan.carpenter@oracle.com]
> Sent: Monday, August 03, 2015 11:47 AM
> To: Woojung Huh - C21699
> Cc: kernel-janitors@vger.kernel.org
> Subject: Re: Microchip's LAN7800 family USB 2/3 to 10/100/1000 Ethernet
> device driver
> 
> There are some other Smatch warnings as well:
> 
> drivers/net/usb/lan78xx.c:2302 lan78xx_start_xmit() error: we previously
> assumed 'skb' could be null (see line 2299)
> skb is dereferenced inside the function call.
> 
> drivers/net/usb/lan78xx.c:2885 lan78xx_bh() info: ignoring unreachable code.
> drivers/net/usb/lan78xx.c:3159 lan78xx_probe() info: ignoring unreachable
> code.
> 
> These are calls to BUG_ON().  You should just delete it, network
> drivers shouldn't call BUG_ON(), at most they should call WARN_ON().
> 
> regards,
> dan carpenter

Re: Microchip's LAN7800 family USB 2/3 to 10/100/1000 Ethernet device driver

[Dan Carpenter]

On Mon, Aug 03, 2015 at 03:48:25PM +0000, Woojung.Huh@microchip.com wrote:
> Hi Dan,
> 
> Thanks for comments.
> I'll look into it and get back to you.
> 
> Where do I post patches?
> 

To netdev and Dave Miller.  But if you could give me a
Reported-by: Dan Carpenter <dan.carpenter@oracle.com> tag, I would
appreciate it.

regards,
dan carpenter

RE: Microchip's LAN7800 family USB 2/3 to 10/100/1000 Ethernet device driver

[Woojung.Huh]

Thanks. I'll do it.

> -----Original Message-----
> From: Dan Carpenter [mailto:dan.carpenter@oracle.com]
> Sent: Monday, August 03, 2015 12:06 PM
> To: Woojung Huh - C21699
> Cc: kernel-janitors@vger.kernel.org
> Subject: Re: Microchip's LAN7800 family USB 2/3 to 10/100/1000 Ethernet
> device driver
> 
> On Mon, Aug 03, 2015 at 03:48:25PM +0000, Woojung.Huh@microchip.com
> wrote:
> > Hi Dan,
> >
> > Thanks for comments.
> > I'll look into it and get back to you.
> >
> > Where do I post patches?
> >
> 
> To netdev and Dave Miller.  But if you could give me a
> Reported-by: Dan Carpenter <dan.carpenter@oracle.com> tag, I would
> appreciate it.
> 
> regards,
> dan carpenter