linux-kernel - Re: [PATCH] irqdomain: remove debugfs_file from struct irq

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:   Thu, 18 Feb 2021 08:16:41 +0100
From:   Greg KH <gregkh@...uxfoundation.org>
To:     Michael Walle <michael@...le.cc>
Cc:     Marc Zyngier <maz@...nel.org>, 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 11:21:22PM +0100, Michael Walle wrote:
> Am 2021-02-17 22:50, schrieb Marc Zyngier:
> > 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?
> 
> Works for me, too. Thanks.

Hm, odd, that shouldn't matter, the debugfs core should be able to
easily handle stuff like this, let me look into that before you apply
this...

thnaks,

greg k-h