netdev - Re: [RFC PATCH 1/7] core/metricfs: Create metricfs, standardized files under debugfs.

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]

Date:   Sat, 8 Aug 2020 07:41:43 +0200
From:   Greg KH <gregkh@...uxfoundation.org>
To:     Jonathan Adams <jwadams@...gle.com>
Cc:     linux-kernel@...r.kernel.org, linux-fsdevel@...r.kernel.org,
        netdev@...r.kernel.org, kvm@...r.kernel.org,
        Paolo Bonzini <pbonzini@...hat.com>,
        Jim Mattson <jmattson@...gle.com>,
        David Rientjes <rientjes@...gle.com>,
        Justin TerAvest <teravest@...gle.com>
Subject: Re: [RFC PATCH 1/7] core/metricfs: Create metricfs, standardized
 files under debugfs.

debugfs interaction nits:

On Fri, Aug 07, 2020 at 02:29:10PM -0700, Jonathan Adams wrote:
> +static struct dentry *metricfs_init_dentry(void)
> +{
> +	static int once;
> +
> +	if (d_metricfs)
> +		return d_metricfs;
> +
> +	if (!debugfs_initialized())
> +		return NULL;
> +
> +	d_metricfs = debugfs_create_dir("metricfs", NULL);
> +
> +	if (!d_metricfs && !once) {

As it is impossible for d_metricfs to ever be NULL, why are you checking
it?

> +		once = 1;
> +		pr_warn("Could not create debugfs directory 'metricfs'\n");

There is a pr_warn_once I think, but again, how can this ever trigger?

> +		return NULL;
> +	}
> +
> +	return d_metricfs;
> +}
> +
> +/* We always cast in and out to struct dentry. */
> +struct metricfs_subsys {
> +	struct dentry dentry;
> +};
> +
> +static struct dentry *metricfs_create_file(const char *name,
> +					   mode_t mode,
> +					   struct dentry *parent,
> +					   void *data,
> +					   const struct file_operations *fops)
> +{
> +	struct dentry *ret;
> +
> +	ret = debugfs_create_file(name, mode, parent, data, fops);
> +	if (!ret)
> +		pr_warn("Could not create debugfs '%s' entry\n", name);

As ret can never be NULL, why check?

There is no need to ever check debugfs return values, just keep on
going, that's the design of it.

thanks,

greg k-h