linux-kernel - Re: [PATCH v15 6/9] ref_tracker: automatically register a file in debugfs for a ref_tracker

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: <6270c853cdf90172d4794e2b601ebc88590b774f.camel@kernel.org>
Date: Thu, 31 Jul 2025 06:29:00 -0400
From: Jeff Layton <jlayton@...nel.org>
To: Kees Cook <kees@...nel.org>
Cc: Andrew Morton <akpm@...ux-foundation.org>, "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>, Maarten Lankhorst <maarten.lankhorst@...ux.intel.com>, 
 Maxime Ripard <mripard@...nel.org>, Thomas Zimmermann
 <tzimmermann@...e.de>, David Airlie <airlied@...il.com>,  Simona Vetter
 <simona@...ll.ch>, Jani Nikula <jani.nikula@...ux.intel.com>, Joonas
 Lahtinen	 <joonas.lahtinen@...ux.intel.com>, Rodrigo Vivi
 <rodrigo.vivi@...el.com>,  Tvrtko Ursulin <tursulin@...ulin.net>, Krzysztof
 Karas <krzysztof.karas@...el.com>, Kuniyuki Iwashima	 <kuniyu@...zon.com>,
 Qasim Ijaz <qasdev00@...il.com>, Nathan Chancellor	 <nathan@...nel.org>,
 Andrew Lunn <andrew@...n.ch>, linux-kernel@...r.kernel.org, 
	netdev@...r.kernel.org, dri-devel@...ts.freedesktop.org, 
	intel-gfx@...ts.freedesktop.org
Subject: Re: [PATCH v15 6/9] ref_tracker: automatically register a file in
 debugfs for a ref_tracker_dir

On Wed, 2025-07-30 at 16:07 -0700, Kees Cook wrote:
> On Wed, Jun 18, 2025 at 10:24:19AM -0400, Jeff Layton wrote:
> > [...]
> > The file is given the name "class@%px", as having the unmodified address
> > is helpful for debugging. This should be safe since this directory is only
> > accessible by root
> > [...]
> > +void ref_tracker_dir_debugfs(struct ref_tracker_dir *dir)
> > +{
> > +	char name[NAME_MAX + 1];
> > +	struct dentry *dentry;
> > +	int ret;
> > +
> > +	/* No-op if already created */
> > +	dentry = xa_load(&debugfs_dentries, (unsigned long)dir);
> > +	if (dentry && !xa_is_err(dentry))
> > +		return;
> > +
> > +	ret = snprintf(name, sizeof(name), "%s@%px", dir->class, dir);
> > +	name[sizeof(name) - 1] = '\0';
> 
> Yikes! Never use %px, and especially don't use it for a stable
> identifier nor expose it to userspace like this. If you absolutely must,
> use %p, but never %px. This is a kernel address leak:
> https://docs.kernel.org/process/deprecated.html#p-format-specifier
> 
> "helpful for debugging" is not a sufficiently good reason; and "only
> accessible by root" has nothing to do with kernel address integrity.
> Those kinds of things are (roughly) managed by various capabilities,
> not DAC uid==0.


>From the link above:

"If you think you can justify it (in comments and commit log) well
enough to stand up to Linus’s scrutiny, maybe you can use “%px”, along
with making sure you have sensible permissions."

Is making it only accessible by root not sensible enough? What are
"sensible permissions" in this instance?

Those questions asked, I'm not dead-set on using %px here. I just
figured it would be more convenient to have the actual address if you
needed to go poke around with drgn. We can change it to %p (or
something else) if it's really a problem.
-- 
Jeff Layton <jlayton@...nel.org>