Re: [PATCH 4/4] net: register debugfs file for net_device refcnt tracker

[Kuniyuki Iwashima <kuniyu@amazon.com>]

From: Andrew Lunn <andrew@lunn.ch>
Date: Tue, 15 Apr 2025 01:16:11 +0200
> On Mon, Apr 14, 2025 at 03:27:36PM -0700, Kuniyuki Iwashima wrote:
> > From: Jeff Layton <jlayton@kernel.org>
> > Date: Mon, 14 Apr 2025 10:45:49 -0400
> > > As a nearly-final step in register_netdevice(), finalize the name in the
> > > refcount tracker, and register a debugfs file for it.
> > > 
> > > Signed-off-by: Jeff Layton <jlayton@kernel.org>
> > > ---
> > >  net/core/dev.c | 2 ++
> > >  1 file changed, 2 insertions(+)
> > > 
> > > diff --git a/net/core/dev.c b/net/core/dev.c
> > > index 2f7f5fd9ffec7c0fc219eb6ba57d57a55134186e..db9cac702bb2230ca2bbc2c04ac0a77482c65fc3 100644
> > > --- a/net/core/dev.c
> > > +++ b/net/core/dev.c
> > > @@ -10994,6 +10994,8 @@ int register_netdevice(struct net_device *dev)
> > >  	    dev->rtnl_link_state == RTNL_LINK_INITIALIZED)
> > >  		rtmsg_ifinfo(RTM_NEWLINK, dev, ~0U, GFP_KERNEL, 0, NULL);
> > >  
> > > +	/* Register debugfs file for the refcount tracker */
> > > +	ref_tracker_dir_debugfs(&dev->refcnt_tracker, dev->name);
> > 
> > dev->name is not unique across network namespaces, so we should specify
> > a netns-specific parent dir here.
> > 
> > For example, syzkaller creates a bunch of devices with the same name in
> > different network namespaces.
> > 
> > Then, we also need to move the file when dev is moved to another netns
> > in __dev_change_net_namespace().
> 
> The address of dev should be unique, and does not change as the netdev
> moves between network name spaces. So you could postfix it with the
> hashed version of an address, as produced by %pK. This is debugfs, it
> does not need to be too friendly.

Fair enough.

Maybe %p should be used ?  I just saw this series.
https://lore.kernel.org/netdev/20250414-restricted-pointers-net-v1-0-12af0ce46cdd@linutronix.de/

Also, it would be nice to update netdev_wait_allrefs_any()
so that we can query the debugfs easily once we find the
possible refcnt leak.

---8<---
diff --git a/net/core/dev.c b/net/core/dev.c
index 93f982de1da0..d129e44ef9f9 100644
--- a/net/core/dev.c
+++ b/net/core/dev.c
@@ -11277,8 +11279,8 @@ static struct net_device *netdev_wait_allrefs_any(struct list_head *list)
 		if (time_after(jiffies, warning_time +
 			       READ_ONCE(netdev_unregister_timeout_secs) * HZ)) {
 			list_for_each_entry(dev, list, todo_list) {
-				pr_emerg("unregister_netdevice: waiting for %s to become free. Usage count = %d\n",
-					 dev->name, netdev_refcnt_read(dev));
+				pr_emerg("unregister_netdevice: waiting for %s (%p) to become free. Usage count = %d\n",
+					 dev->name, dev, netdev_refcnt_read(dev));
 				ref_tracker_dir_print(&dev->refcnt_tracker, 10);
 			}
 
---8<---