netdev.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Dan Carpenter <dan.carpenter@oracle.com>
To: stefan.sorensen@spectralink.com
Cc: netdev@vger.kernel.org
Subject: [bug report] dp83640: Delay scheduled work.
Date: Tue, 28 Feb 2017 09:36:12 +0300	[thread overview]
Message-ID: <20170228063612.GA25830@mwanda> (raw)

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

                 reply	other threads:[~2017-02-28  6:47 UTC|newest]

Thread overview: [no followups] expand[flat|nested]  mbox.gz  Atom feed

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20170228063612.GA25830@mwanda \
    --to=dan.carpenter@oracle.com \
    --cc=netdev@vger.kernel.org \
    --cc=stefan.sorensen@spectralink.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).