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

[Jeff Layton <jlayton@kernel.org>]

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@kernel.org>