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 for Android: free password hash cracker in your pocket

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

Message-ID: <23f93f84e000ebee28614bf85a4013648fa66a00.camel@kernel.org>
Date: Sun, 13 Apr 2025 07:40:59 -0400
From: Jeff Layton <jlayton@...nel.org>
To: Andrew Lunn <andrew@...n.ch>
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

On Thu, 2025-04-10 at 16:12 +0200, Andrew Lunn wrote:
> > Oh, ok. I guess you mean these names?
> > 
> >         ref_tracker_dir_init(&net->refcnt_tracker, 128, "net refcnt");
> >         ref_tracker_dir_init(&net->notrefcnt_tracker, 128, "net notrefcnt");
> > 
> > Two problems there:
> > 
> > 1/ they have an embedded space in the name which is just painful. Maybe we can replace those with underscores?
> > 2/ they aren't named in a per-net namespace way
> 
> So the first question is, are the names ABI? Are they exposed to
> userspace anywhere? Can we change them?
> 
> If we can change them, space to _ is a simple change. Another option
> is what hwmon does, hwmon_sanitize_name() which turns a name into
> something which is legal in a filesystem. If all of this code can be
> pushed into the core tracker, so all trackers appear in debugfs, such
> a sanitiser is the way i would go.
> 
> And if we can change the name, putting the netns into the name would
> also work. There is then no need for the directory, if they have
> unique names.
> 
> Looking at other users of ref_tracker_dir_init():
> 
> ~/linux$ grep -r ref_tracker_dir_init
> lib/test_ref_tracker.c:	ref_tracker_dir_init(&ref_dir, 100, "selftest");
> 
> Can only be loaded once, so is unique.
>
> drivers/gpu/drm/i915/intel_wakeref.c:	ref_tracker_dir_init(&wf->debug, INTEL_REFTRACK_DEAD_COUNT, name);
> 
> Looks like it is unique for one GPU, but if you have multiple GPUs
> they are not unique.
> 

We'll need some input from the i915 folks then.

> drivers/gpu/drm/i915/intel_runtime_pm.c:	ref_tracker_dir_init(&rpm->debug, INTEL_REFTRACK_DEAD_COUNT, dev_name(rpm->kdev));
> 
> At a guess kdev is unique.
> 

Yeah, looks like it.

> drivers/gpu/drm/display/drm_dp_tunnel.c:	ref_tracker_dir_init(&mgr->ref_tracker, 16, "dptun");
> 
> Probably not unique.
> 

Looking more here, perhaps we can incorporate mgr->dev->unique into the
name? I don't know much about dp drivers. Can multiple mgrs point to
the same device?

> 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");
> 
> Not unique across name spaces, but ...

That's easily fixable. net->ns.inum is unique.

> 
> So could the tracker core check if the debugfs file already exists,
> emit a warning if it does, and keep going? I think debugfs_lookup()
> will tell you if a file already exists, or debugfs_create_file() will
> return -EEXIST, which is probably safer, no race condition.
> 

Yeah, that should be possible. I think too that we can eliminate the
buffer management in this codepath by allowing pr_ostream() to output
directly to a seq_file. I'll look into that as well.

Thanks for the input!
-- 
Jeff Layton <jlayton@...nel.org>