kernel/irq/irqdomain.c: fix memory leak with using debugfs_lookup()

Message ID 20230202151554.2310273-1-gregkh@linuxfoundation.org
State New
Headers
Series kernel/irq/irqdomain.c: fix memory leak with using debugfs_lookup() |

Commit Message

Greg KH Feb. 2, 2023, 3:15 p.m. UTC
  When calling debugfs_lookup() the result must have dput() called on it,
otherwise the memory will leak over time.  To make things simpler, just
call debugfs_lookup_and_remove() instead which handles all of the logic
at once.

Cc: Marc Zyngier <maz@kernel.org>
Cc: Thomas Gleixner <tglx@linutronix.de>
Cc: linux-kernel@vger.kernel.org
Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
---
 kernel/irq/irqdomain.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
  

Comments

Marc Zyngier Feb. 2, 2023, 4:48 p.m. UTC | #1
On Thu, 02 Feb 2023 15:15:54 +0000,
Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:
> 
> When calling debugfs_lookup() the result must have dput() called on it,
> otherwise the memory will leak over time.  To make things simpler, just
> call debugfs_lookup_and_remove() instead which handles all of the logic
> at once.
> 
> Cc: Marc Zyngier <maz@kernel.org>
> Cc: Thomas Gleixner <tglx@linutronix.de>
> Cc: linux-kernel@vger.kernel.org
> Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> ---
>  kernel/irq/irqdomain.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
> index 8fe1da9614ee..e2096b51c004 100644
> --- a/kernel/irq/irqdomain.c
> +++ b/kernel/irq/irqdomain.c
> @@ -1915,7 +1915,7 @@ 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));
> +	debugfs_lookup_and_remove(d->name, domain_dir);
>  }
>  
>  void __init irq_domain_debugfs_init(struct dentry *root)

Reviewed-by: Marc Zyngier <maz@kernel.org>

Maybe add a Cc stable to get this in 6.1?

Thanks,

	M.
  
Greg KH Feb. 2, 2023, 4:51 p.m. UTC | #2
On Thu, Feb 02, 2023 at 04:48:30PM +0000, Marc Zyngier wrote:
> On Thu, 02 Feb 2023 15:15:54 +0000,
> Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:
> > 
> > When calling debugfs_lookup() the result must have dput() called on it,
> > otherwise the memory will leak over time.  To make things simpler, just
> > call debugfs_lookup_and_remove() instead which handles all of the logic
> > at once.
> > 
> > Cc: Marc Zyngier <maz@kernel.org>
> > Cc: Thomas Gleixner <tglx@linutronix.de>
> > Cc: linux-kernel@vger.kernel.org
> > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > ---
> >  kernel/irq/irqdomain.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
> > index 8fe1da9614ee..e2096b51c004 100644
> > --- a/kernel/irq/irqdomain.c
> > +++ b/kernel/irq/irqdomain.c
> > @@ -1915,7 +1915,7 @@ 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));
> > +	debugfs_lookup_and_remove(d->name, domain_dir);
> >  }
> >  
> >  void __init irq_domain_debugfs_init(struct dentry *root)
> 
> Reviewed-by: Marc Zyngier <maz@kernel.org>
> 
> Maybe add a Cc stable to get this in 6.1?

Sure, I can, but how often are irq domains removed from the system under
a normal operation?

Or you can take it through your tree and add that?  I too can take it
through mine, which ever is easier for you.

thanks,

greg k-h
  
Marc Zyngier Feb. 2, 2023, 4:56 p.m. UTC | #3
On Thu, 02 Feb 2023 16:51:50 +0000,
Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:
> 
> On Thu, Feb 02, 2023 at 04:48:30PM +0000, Marc Zyngier wrote:
> > On Thu, 02 Feb 2023 15:15:54 +0000,
> > Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:
> > > 
> > > When calling debugfs_lookup() the result must have dput() called on it,
> > > otherwise the memory will leak over time.  To make things simpler, just
> > > call debugfs_lookup_and_remove() instead which handles all of the logic
> > > at once.
> > > 
> > > Cc: Marc Zyngier <maz@kernel.org>
> > > Cc: Thomas Gleixner <tglx@linutronix.de>
> > > Cc: linux-kernel@vger.kernel.org
> > > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > > ---
> > >  kernel/irq/irqdomain.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
> > > index 8fe1da9614ee..e2096b51c004 100644
> > > --- a/kernel/irq/irqdomain.c
> > > +++ b/kernel/irq/irqdomain.c
> > > @@ -1915,7 +1915,7 @@ 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));
> > > +	debugfs_lookup_and_remove(d->name, domain_dir);
> > >  }
> > >  
> > >  void __init irq_domain_debugfs_init(struct dentry *root)
> > 
> > Reviewed-by: Marc Zyngier <maz@kernel.org>
> > 
> > Maybe add a Cc stable to get this in 6.1?
> 
> Sure, I can, but how often are irq domains removed from the system under
> a normal operation?

As many time as you want if you're doing virtualisation and have the
right sort of HW (anything GICv4.x, for example, which will create
per-VM irqdomains).

> Or you can take it through your tree and add that?  I too can take
> it through mine, which ever is easier for you.

Sure, I can queue that, although that will probably be next week.

Thanks,

	M.
  
Greg KH Feb. 3, 2023, 6:04 a.m. UTC | #4
On Thu, Feb 02, 2023 at 04:56:50PM +0000, Marc Zyngier wrote:
> On Thu, 02 Feb 2023 16:51:50 +0000,
> Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:
> > 
> > On Thu, Feb 02, 2023 at 04:48:30PM +0000, Marc Zyngier wrote:
> > > On Thu, 02 Feb 2023 15:15:54 +0000,
> > > Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote:
> > > > 
> > > > When calling debugfs_lookup() the result must have dput() called on it,
> > > > otherwise the memory will leak over time.  To make things simpler, just
> > > > call debugfs_lookup_and_remove() instead which handles all of the logic
> > > > at once.
> > > > 
> > > > Cc: Marc Zyngier <maz@kernel.org>
> > > > Cc: Thomas Gleixner <tglx@linutronix.de>
> > > > Cc: linux-kernel@vger.kernel.org
> > > > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org>
> > > > ---
> > > >  kernel/irq/irqdomain.c | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > 
> > > > diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
> > > > index 8fe1da9614ee..e2096b51c004 100644
> > > > --- a/kernel/irq/irqdomain.c
> > > > +++ b/kernel/irq/irqdomain.c
> > > > @@ -1915,7 +1915,7 @@ 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));
> > > > +	debugfs_lookup_and_remove(d->name, domain_dir);
> > > >  }
> > > >  
> > > >  void __init irq_domain_debugfs_init(struct dentry *root)
> > > 
> > > Reviewed-by: Marc Zyngier <maz@kernel.org>
> > > 
> > > Maybe add a Cc stable to get this in 6.1?
> > 
> > Sure, I can, but how often are irq domains removed from the system under
> > a normal operation?
> 
> As many time as you want if you're doing virtualisation and have the
> right sort of HW (anything GICv4.x, for example, which will create
> per-VM irqdomains).

Ok, I'll queue this up now and get it to Linus for 6.2-final.

thanks,

greg k-h
  

Patch

diff --git a/kernel/irq/irqdomain.c b/kernel/irq/irqdomain.c
index 8fe1da9614ee..e2096b51c004 100644
--- a/kernel/irq/irqdomain.c
+++ b/kernel/irq/irqdomain.c
@@ -1915,7 +1915,7 @@  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));
+	debugfs_lookup_and_remove(d->name, domain_dir);
 }
 
 void __init irq_domain_debugfs_init(struct dentry *root)