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: <YC4X4iLMCK3tNVsF@kroah.com>
Date:   Thu, 18 Feb 2021 08:31:46 +0100
From:   Greg KH <gregkh@...uxfoundation.org>
To:     Marc Zyngier <maz@...nel.org>, Michael Walle <michael@...le.cc>
Cc:     linux-kernel@...r.kernel.org, tglx@...utronix.de
Subject: Re: [PATCH] irqdomain: remove debugfs_file from struct irq_domain

On Wed, Feb 17, 2021 at 09:50:38PM +0000, Marc Zyngier wrote:
> On Wed, 17 Feb 2021 20:10:50 +0000,
> Michael Walle <michael@...le.cc> wrote:
> > 
> > Am 2021-02-17 21:02, schrieb Marc Zyngier:
> > > On 2021-02-17 19:57, Michael Walle wrote:
> > >> Hi Greg,
> > >> 
> > >>> There's no need to keep around a dentry pointer to a simple file that
> > >>> debugfs itself can look up when we need to remove it from the system.
> > >>> So simplify the code by deleting the variable and cleaning up the
> > >>> logic
> > >>> around the debugfs file.
> > >> 
> > >> This will generate the following oops on my board (arm64,
> > >> freescale/fsl-ls1028a-kontron-sl28-var3-ads2.dts). In debugfs_lookup()
> > >> debugfs_mount is NULL.
> > > 
> > > That's odd. I gave it a go yesterday, and nothing blew up.
> > > Which makes me wonder whether I had the debug stuff enabled
> > > the first place...
> > > 
> > > I've dropped the patch from -next for now until I figure it out
> > > (probably tomorrow).
> > 
> > Mh, maybe its my .config, I've attached it. I also noticed that
> > the board boots just fine in our kernel-ci [1].
> 
> I reproduced here. I had disabled GENERIC_IRQ_DEBUGFS for obscure
> reasons, and it caught fire as I re-enabled it.
> 
> Adding this fixes it for me:
> 
> diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
> index 367ff1c35f75..d8a14cf1a7b6 100644
> --- a/kernel/irq/irqdomain.c
> +++ b/kernel/irq/irqdomain.c
> @@ -1904,7 +1904,8 @@ static void debugfs_add_domain_dir(struct irq_domain *d)
>  
>  static void debugfs_remove_domain_dir(struct irq_domain *d)
>  {
> -	debugfs_remove(debugfs_lookup(d->name, domain_dir));
> +	if (domain_dir)
> +		debugfs_remove(debugfs_lookup(d->name, domain_dir));
>  }
>  
>  void __init irq_domain_debugfs_init(struct dentry *root)
> 
> 
> Could you please check whether it works for you?

Can you try this debugfs core change instead?  Callers to debugfs should
not have to do the above type of checking as debugfs should be much more
robust than that.

thanks,

greg k-h


diff --git a/fs/debugfs/inode.c b/fs/debugfs/inode.c
index 2fcf66473436..5aa798f52b6e 100644
--- a/fs/debugfs/inode.c
+++ b/fs/debugfs/inode.c
@@ -297,7 +297,7 @@ struct dentry *debugfs_lookup(const char *name, struct dentry *parent)
 {
 	struct dentry *dentry;
 
-	if (IS_ERR(parent))
+	if (IS_ERR_OR_NULL(name) || IS_ERR(parent))
 		return NULL;
 
 	if (!parent)

Powered by blists - more mailing lists

Powered by Openwall GNU/*/Linux Powered by OpenVZ