linux-kernel - Re: [PATCH] iio: core: use debugfs_remove

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: <4728487938a37207f20cc5d4cb6fbbdd04f56358.camel@analog.com>
Date:   Mon, 16 Dec 2019 07:43:30 +0000
From:   "Ardelean, Alexandru" <alexandru.Ardelean@...log.com>
To:     "jic23@...nel.org" <jic23@...nel.org>
CC:     "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "linux-iio@...r.kernel.org" <linux-iio@...r.kernel.org>,
        "lars@...afoo.de" <lars@...afoo.de>
Subject: Re: [PATCH] iio: core: use debugfs_remove_recursive() on IIO unload

On Sun, 2019-12-15 at 16:25 +0000, Jonathan Cameron wrote:
> [External]
> 
> On Wed, 11 Dec 2019 16:59:03 +0200
> Alexandru Ardelean <alexandru.ardelean@...log.com> wrote:
> 
> > From: Lars-Peter Clausen <lars@...afoo.de>
> > 
> > The debugfs dir may have multiple files/dirs attached to it. Use
> > debugfs_remove_recursive() to clean it up when the IIO module unloads.
> > 
> > Signed-off-by: Lars-Peter Clausen <lars@...afoo.de>
> > Signed-off-by: Alexandru Ardelean <alexandru.ardelean@...log.com>
> > ---
> > 
> > Note: note sure if this needs a Fixes tag. It's been in here for ages
> > now,
> >       so it may not be a big issue.
> 
> There is some argument in favour of doing this as a defensive measure,
> but
> in current IIO debugfs directories are only created by drivers that call
> (indirectly) iio_device_register_debugfs and those must call
> (again indirectly) iio_device_unregister_debugfs which has called
> the recursive free on everything in their directories.  So unless
> something
> very odd is going on this isn't a bug.
> 
> Am I missing something?

So, I admit my note is open for interpretation.
[ Rergarding the "Fixes" tag, I'm still not super-clear on when/why/how
it's needed. I'll get there soon though. ]

This patch has been in our tree since 2012, and I'm not sure why it's using
debugfs_remove_recursive() vs plain debugfs_remove().

I sometimes take a look at these old patches, try to make some sense of
them and upstream them, or override them with the version of upstream code.
My intent is to sync ADI code [for IIO] closer to upstream. Which would
make it easier to track any divergence.

We're not seeing any bug with/without this.
I guess my note about the "Fixes" tag may be a bit clumsy.

From my side, both debugfs_remove_recursive() & plain debugfs_remove() are
fine.

Thanks
Alex

> 
> 
> >  drivers/iio/industrialio-core.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/iio/industrialio-core.c
> > b/drivers/iio/industrialio-core.c
> > index 9a3579943574..dab67cb69fe6 100644
> > --- a/drivers/iio/industrialio-core.c
> > +++ b/drivers/iio/industrialio-core.c
> > @@ -292,7 +292,7 @@ static void __exit iio_exit(void)
> >  	if (iio_devt)
> >  		unregister_chrdev_region(iio_devt, IIO_DEV_MAX);
> >  	bus_unregister(&iio_bus_type);
> > -	debugfs_remove(iio_debugfs_dentry);
> > +	debugfs_remove_recursive(iio_debugfs_dentry);
> >  }
> >  
> >  #if defined(CONFIG_DEBUG_FS)