linux-kernel - Re: [PATCH] gpio: virtuser: remove debugfs_create

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] [day] [month] [year] [list]

Message-ID: <aJnJADCjzP5h0ZMf@bhairav-test.ee.iitb.ac.in>
Date: Mon, 11 Aug 2025 16:12:08 +0530
From: Akhilesh Patil <akhilesh@...iitb.ac.in>
To: Bartosz Golaszewski <brgl@...ev.pl>
Cc: linus.walleij@...aro.org, dianders@...omium.org,
	linux-gpio@...r.kernel.org, linux-kernel@...r.kernel.org,
	akhileshpatilvnit@...il.com, skhan@...uxfoundation.org
Subject: Re: [PATCH] gpio: virtuser: remove debugfs_create_dir() error checks

On Mon, Aug 11, 2025 at 09:47:13AM +0200, Bartosz Golaszewski wrote:
> On Sat, Aug 9, 2025 at 6:25 PM Akhilesh Patil <akhilesh@...iitb.ac.in> wrote:
> >
> > Remove return value checks for debugfs_create_dir() wherever
> > appropriate. Follow guidelines mentioned in [1] that callers
> > should ignore errors returned as other debugfs functions handle them
> > appropriately.
> > Refer commit 8bcbde2bb1374 ("debugfs: Document that debugfs_create
> > functions need not be error checked") to clean up unnecessary error checks
> > without impacting the functionality.
> >
> > Fixes: 91581c4b3f29e ("gpio: virtuser: new virtual testing driver for the GPIO API")
> > Link: https://lore.kernel.org/all/20220222154555.1.I26d364db7a007f8995e8f0dac978673bc8e9f5e2@changeid/ [1]
> > Signed-off-by: Akhilesh Patil <akhilesh@...iitb.ac.in>
> > ---
> 
> The commit you linked says: "In many cases (...)". This is not one of
> these cases as this driver is completely useless without functional
> debugfs entries so it very much makes sense to check the relevant
> calls.

Agree. gpio_virtuser_dbgfs_init_line_array_attrs() should return error
upon failure to create debugfs interface.
However with my interpretation of commit 8bcbde2bb1374, in this case we
can still skip error checks for  debugfs_create_dir() at [1] as any error
returned by it will be handled in gpio_virtuser_create_debugfs_attrs() at [2]. 
debugfs_create_file() at [3] can accept error dentry and do not crash.
So IMO, effectively we stil return error upon failure to create debugfs
dir and files while following guidelines mentioned in the commit we
are reffering. I have intentionally not removed [2] and removed [1] with
this thought. 
	Thank you for your time reviewing it :) 

	data->ad.dbgfs_dir = debugfs_create_dir(name, dbgfs_entry);
[1] -->	if (IS_ERR(data->ad.dbgfs_dir))
		return PTR_ERR(data->ad.dbgfs_dir);

	return gpio_virtuser_create_debugfs_attrs(
			gpio_virtuser_line_array_dbgfs_attrs,
			ARRAY_SIZE(gpio_virtuser_line_array_dbgfs_attrs),
			data->ad.dbgfs_dir, data);




static int gpio_virtuser_create_debugfs_attrs(
			const struct gpio_virtuser_dbgfs_attr_descr *attr,
			size_t num_attrs, struct dentry *parent, void *data)
{
	struct dentry *ret;
	size_t i;

	for (i = 0; i < num_attrs; i++, attr++) {
[3] -->		ret = debugfs_create_file(attr->name, 0644, parent, data,
					  attr->fops);
[2] -->		if (IS_ERR(ret))
			return PTR_ERR(ret);
	}

	return 0;
}


> 
> Bart

Regards,
Akhilesh