net: add net device refcount tracker infrastructure

net device are refcounted. Over the years we had numerous bugs
caused by imbalanced dev_hold() and dev_put() calls.

The general idea is to be able to precisely pair each decrement with
a corresponding prior increment. Both share a cookie, basically
a pointer to private data storing stack traces.

This patch adds dev_hold_track() and dev_put_track().

To use these helpers, each data structure owning a refcount
should also use a "netdevice_tracker" to pair the hold and put.

netdevice_tracker dev_tracker;
...
dev_hold_track(dev, &dev_tracker, GFP_ATOMIC);
...
dev_put_track(dev, &dev_tracker);

Whenever a leak happens, we will get precise stack traces
of the point dev_hold_track() happened, at device dismantle phase.

We will also get a stack trace if too many dev_put_track() for the same
netdevice_tracker are attempted.

This is guarded by CONFIG_NET_DEV_REFCNT_TRACKER option.

Signed-off-by: Eric Dumazet <edumazet@google.com>
Signed-off-by: Jakub Kicinski <kuba@kernel.org>

diff --git a/include/linux/netdevice.h b/include/linux/netdevice.h
index 65117f01d5f2a9c9e815b6c967d5e9e4c94af0ae..143d60ed004732e4a086e66fdcf7b3d362c1dc20 100644 (file)
--- a/include/linux/netdevice.h
+++ b/include/linux/netdevice.h
@@ -48,6 +48,7 @@
 #include <uapi/linux/pkt_cls.h>
 #include <linux/hashtable.h>
 #include <linux/rbtree.h>
+#include <linux/ref_tracker.h>
 
 struct netpoll_info;
 struct device;
@@ -300,6 +301,12 @@ enum netdev_state_t {
 };
 
 
+#ifdef CONFIG_NET_DEV_REFCNT_TRACKER
+typedef struct ref_tracker *netdevice_tracker;
+#else
+typedef struct {} netdevice_tracker;
+#endif
+
 struct gro_list {
        struct list_head        list;
        int                     count;
@@ -1865,6 +1872,7 @@ enum netdev_ml_priv_type {
  *     @proto_down_reason:     reason a netdev interface is held down
  *     @pcpu_refcnt:           Number of references to this device
  *     @dev_refcnt:            Number of references to this device
+ *     @refcnt_tracker:        Tracker directory for tracked references to this device
  *     @todo_list:             Delayed register/unregister
  *     @link_watch_list:       XXX: need comments on this one
  *
@@ -2178,6 +2186,7 @@ struct net_device {
 #else
        refcount_t              dev_refcnt;
 #endif
+       struct ref_tracker_dir  refcnt_tracker;
 
        struct list_head        link_watch_list;
 
@@ -3805,6 +3814,7 @@ void netdev_run_todo(void);
  *     @dev: network device
  *
  * Release reference to device to allow it to be freed.
+ * Try using dev_put_track() instead.
  */
 static inline void dev_put(struct net_device *dev)
 {
@@ -3822,6 +3832,7 @@ static inline void dev_put(struct net_device *dev)
  *     @dev: network device
  *
  * Hold reference to device to keep it from being freed.
+ * Try using dev_hold_track() instead.
  */
 static inline void dev_hold(struct net_device *dev)
 {
@@ -3834,6 +3845,40 @@ static inline void dev_hold(struct net_device *dev)
        }
 }
 
+static inline void netdev_tracker_alloc(struct net_device *dev,
+                                       netdevice_tracker *tracker, gfp_t gfp)
+{
+#ifdef CONFIG_NET_DEV_REFCNT_TRACKER
+       ref_tracker_alloc(&dev->refcnt_tracker, tracker, gfp);
+#endif
+}
+
+static inline void netdev_tracker_free(struct net_device *dev,
+                                      netdevice_tracker *tracker)
+{
+#ifdef CONFIG_NET_DEV_REFCNT_TRACKER
+       ref_tracker_free(&dev->refcnt_tracker, tracker);
+#endif
+}
+
+static inline void dev_hold_track(struct net_device *dev,
+                                 netdevice_tracker *tracker, gfp_t gfp)
+{
+       if (dev) {
+               dev_hold(dev);
+               netdev_tracker_alloc(dev, tracker, gfp);
+       }
+}
+
+static inline void dev_put_track(struct net_device *dev,
+                                netdevice_tracker *tracker)
+{
+       if (dev) {
+               netdev_tracker_free(dev, tracker);
+               dev_put(dev);
+       }
+}
+
 /* Carrier loss detection, dial on demand. The functions netif_carrier_on
  * and _off may be called from IRQ context, but it is caller
  * who is responsible for serialization of these calls.

diff --git a/lib/Kconfig.debug b/lib/Kconfig.debug
index 633c2c5cb45bd435a7684dbe2f2eca477c871463..6504b97f8dfd786b6a7b9cdb2c2eda5aaa2c2b03 100644 (file)
--- a/lib/Kconfig.debug
+++ b/lib/Kconfig.debug
@@ -598,6 +598,11 @@ config DEBUG_MISC
          Say Y here if you need to enable miscellaneous debug code that should
          be under a more specific debug option but isn't.
 
+menu "Networking Debugging"
+
+source "net/Kconfig.debug"
+
+endmenu # "Networking Debugging"
 
 menu "Memory Debugging"
 

diff --git a/net/Kconfig.debug b/net/Kconfig.debug
new file mode 100644 (file)
index 0000000..fb5c70e
--- /dev/null
+++ b/net/Kconfig.debug
@@ -0,0 +1,10 @@
+# SPDX-License-Identifier: GPL-2.0-only
+
+config NET_DEV_REFCNT_TRACKER
+       bool "Enable net device refcount tracking"
+       depends on DEBUG_KERNEL && STACKTRACE_SUPPORT
+       select REF_TRACKER
+       default n
+       help
+         Enable debugging feature to track device references.
+         This adds memory and cpu costs.

diff --git a/net/core/dev.c b/net/core/dev.c
index aba8acc1238c2213b9fc0c47c93568f731f375e5..1740d6cfe86b58359cceaec7ee9cc015a3843723 100644 (file)
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -9864,6 +9864,7 @@ static void netdev_wait_allrefs(struct net_device *dev)
                               netdev_unregister_timeout_secs * HZ)) {
                        pr_emerg("unregister_netdevice: waiting for %s to become free. Usage count = %d\n",
                                 dev->name, refcnt);
+                       ref_tracker_dir_print(&dev->refcnt_tracker, 10);
                        warning_time = jiffies;
                }
        }
@@ -10154,6 +10155,7 @@ struct net_device *alloc_netdev_mqs(int sizeof_priv, const char *name,
        dev = PTR_ALIGN(p, NETDEV_ALIGN);
        dev->padded = (char *)dev - (char *)p;
 
+       ref_tracker_dir_init(&dev->refcnt_tracker, 128);
 #ifdef CONFIG_PCPU_DEV_REFCNT
        dev->pcpu_refcnt = alloc_percpu(int);
        if (!dev->pcpu_refcnt)
@@ -10270,6 +10272,7 @@ void free_netdev(struct net_device *dev)
        list_for_each_entry_safe(p, n, &dev->napi_list, dev_list)
                netif_napi_del(p);
 
+       ref_tracker_dir_exit(&dev->refcnt_tracker);
 #ifdef CONFIG_PCPU_DEV_REFCNT
        free_percpu(dev->pcpu_refcnt);
        dev->pcpu_refcnt = NULL;