All of lore.kernel.org
 help / color / mirror / Atom feed
* [bug report] dp83640: Delay scheduled work.
@ 2017-02-28  6:36 Dan Carpenter
  0 siblings, 0 replies; only message in thread
From: Dan Carpenter @ 2017-02-28  6:36 UTC (permalink / raw)
  To: stefan.sorensen; +Cc: netdev

Hello Stefan Sørensen,

The patch 4b063258ab93: "dp83640: Delay scheduled work." from Nov 3,
2015, leads to the following static checker warning:

	drivers/net/phy/dp83640.c:1442 dp83640_rxtstamp()
	warn: 'skb' was already freed.

drivers/net/phy/dp83640.c
  1402          struct dp83640_skb_info *skb_info = (struct dp83640_skb_info *)skb->cb;
  1403          struct list_head *this, *next;
  1404          struct rxts *rxts;
  1405          struct skb_shared_hwtstamps *shhwtstamps = NULL;
                                             ^^^^^^^^^^^^^^^^^^
  1406          unsigned long flags;
  1407  
  1408          if (is_status_frame(skb, type)) {
  1409                  decode_status_frame(dp83640, skb);
  1410                  kfree_skb(skb);
  1411                  return true;
  1412          }
  1413  
  1414          if (!dp83640->hwts_rx_en)
  1415                  return false;
  1416  
  1417          if ((type & dp83640->version) == 0 || (type & dp83640->layer) == 0)
  1418                  return false;
  1419  
  1420          spin_lock_irqsave(&dp83640->rx_lock, flags);
  1421          prune_rx_ts(dp83640);
  1422          list_for_each_safe(this, next, &dp83640->rxts) {
  1423                  rxts = list_entry(this, struct rxts, list);
  1424                  if (match(skb, type, rxts)) {
  1425                          shhwtstamps = skb_hwtstamps(skb);
  1426                          memset(shhwtstamps, 0, sizeof(*shhwtstamps));
  1427                          shhwtstamps->hwtstamp = ns_to_ktime(rxts->ns);
  1428                          netif_rx_ni(skb);
                                ^^^^^^^^^^^^^^^
If shhwtstamps is non-NULL then we call netif_rx_ni(skb);.  If this
call returns NET_RX_DROP then that means we've done a kfree_skb(skb).

  1429                          list_del_init(&rxts->list);
  1430                          list_add(&rxts->list, &dp83640->rxpool);
  1431                          break;
  1432                  }
  1433          }
  1434          spin_unlock_irqrestore(&dp83640->rx_lock, flags);
  1435  
  1436          if (!shhwtstamps) {
  1437                  skb_info->ptp_type = type;
  1438                  skb_info->tmo = jiffies + SKB_TIMESTAMP_TIMEOUT;
  1439                  skb_queue_tail(&dp83640->rx_queue, skb);
  1440                  schedule_delayed_work(&dp83640->ts_work, SKB_TIMESTAMP_TIMEOUT);
  1441          } else {
  1442                  netif_rx_ni(skb);


And then we call it a second time outside the spinlock.  When I look at
the commit which added this, it feels like something that was added by
mistake.  But I'm really familiar enough with this code to say if I
haven't missed something.

  1443          }
  1444  
  1445          return true;
  1446  }

regards,
dan carpenter

^ permalink raw reply	[flat|nested] only message in thread

only message in thread, other threads:[~2017-02-28  6:47 UTC | newest]

Thread overview: (only message) (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2017-02-28  6:36 [bug report] dp83640: Delay scheduled work Dan Carpenter

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.