Skip to contentSkip to conversation

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

DISCUSSION Dan Carpenter 2 participants 5 messages Aug 3, 2015 Kernel janitors

lore
Share & tools
lore
#0 lore raw

Hello [email protected],

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

#1 lore raw

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

#2 lore raw
In reply to Dan Carpenter

Hi Dan,

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

Where do I post patches?

Thanks,
Woojung

Quoted 23 lines

-----Original Message-----
From: Dan Carpenter [mailto:[email protected]]
Sent: Monday, August 03, 2015 11:47 AM
To: Woojung Huh - C21699
Cc: [email protected]
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

#4 lore raw
In reply to Dan Carpenter

Thanks. I'll do it.

Quoted 24 lines

-----Original Message-----
From: Dan Carpenter [mailto:[email protected]]
Sent: Monday, August 03, 2015 12:06 PM
To: Woojung Huh - C21699
Cc: [email protected]
Subject: Re: Microchip's LAN7800 family USB 2/3 to 10/100/1000 Ethernet device driver

Quoted 7 lines · On Mon, Aug 03, 2015 at 03:48:25PM +0000, [email protected] 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

appreciate it.

regards,
dan carpenter