linux-kernel - Re: [PATCH v2 2/2] net: add debugfs files for showing netns refcount tracking info

lists.openwall.net		lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC
Open Source and information security mailing list archives

Hash Suite: Windows password security audit tool. GUI, reports in PDF.

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Message-ID: <1bc3164b-0e41-49e1-9ed2-cf37997ab53e@lunn.ch>
Date: Mon, 14 Apr 2025 14:46:52 +0200
From: Andrew Lunn <andrew@...n.ch>
To: Jeff Layton <jlayton@...nel.org>
Cc: "David S. Miller" <davem@...emloft.net>,
	Eric Dumazet <edumazet@...gle.com>,
	Jakub Kicinski <kuba@...nel.org>, Paolo Abeni <pabeni@...hat.com>,
	Simon Horman <horms@...nel.org>,
	Andrew Morton <akpm@...ux-foundation.org>, netdev@...r.kernel.org,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 2/2] net: add debugfs files for showing netns refcount
 tracking info

> > > We'll need some input from the i915 folks then.
> > 
> > That is why i think it would be better to add a warning, give the i915
> > folks a heads up, and leave them to fix it how they want. We want the
> > warning anyway to cover new refcount instances being added.
> > 
> 
> There will definitely be a pr_warn() if creation fails. I was hoping to
> send a suggested change alongside the patchset, but I may have to just
> leave it up to them.
> 
> I threw together a draft patchset that auto-registers a debugfs file
> for every call to ref_tracker_dir_init(). The problem I've hit now
> though is that at least in the networking cases, the kernel calls
> ref_tracker_dir_init() very early in the process of creating a new
> objects, so we're not getting good names here:
> 
> The "name" in alloc_netdev_mqs is actually a format string, so we end
> up with a name like "eth%d" here:
> 
> net/core/dev.c: ref_tracker_dir_init(&dev->refcnt_tracker, 128, name);
 
Oh, yes, never thought about that...

It also seems like it might be a common problem, the subsystem defined
name has not been given yet, although the core dev_name(dev) should
work. But in netdev drivers, alloc_netdev_mqs() does not have access
to that.

> At the point that we call these (preinit), net->ns.inum hasn't been
> assigned yet, so we can't incorporate it properly into the dentry name:
> 
> net/core/net_namespace.c:       ref_tracker_dir_init(&net->refcnt_tracker, 128, "net refcnt");
> net/core/net_namespace.c:       ref_tracker_dir_init(&net->notrefcnt_tracker, 128, "net notrefcnt");
> 
> We could do the ref_tracker_dir_init() calls later, but we may end up
> missing some references that way (or end up crashing because the "dir"
> isn't initialized.
> 
> My current thinking is to add a new ref_tracker_dir_finalize() function
> that we could use to finalize the name and register the debugfs files
> after we have the requisite info. It's an extra manual step, but I like
> that better than moving around the ref_tracker_dir_init() calls.

Agree. Although, bike shedding, i don't really like the _finalize in
ref_tracker_dir_finalize(). Ideally this should be optional and
populate debugfs.  _finalize does not sound particularly optional. So
maybe ref_tracker_dir_debugfs()?

This also has the advantage of we need to worry less about GPU
drivers. They see no change in behaviour, but we can give them a heads
up the new call exists if they want to use it to populate debugfs.

	Andrew