Skip to contentSkip to conversation

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

PATCH In review Greg Kroah-Hartman 2 participants 5 messages Feb 2, 2023 – Feb 3, 2023

Subsystems (2)
lore
Share & tools
lore
#0 lore raw

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

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.

---
 kernel/irq/irqdomain.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
kernel/irq/irqdomain.c +1 -1
index 8fe1da9614ee..e2096b51c004 100644
@@ -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)
Signature · 1 line
2.39.1
Reviewed-by on v1 1/1
#1 lore raw
Quoted 19 lines · Greg Kroah-Hartman

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.

---
 kernel/irq/irqdomain.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
kernel/irq/irqdomain.c +0 -0
index 8fe1da9614ee..e2096b51c004 100644

kernel/irq/irqdomain.c static void debugfs_add_domain_dir(struct irq_domain *d) · line 1921

Quoted 8 lines
kernel/irq/irqdomain.c +1 -1
@@ -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)

Maybe add a Cc stable to get this in 6.1?

Thanks,

	M.
Signature · 1 line
Without deviation from the norm, progress is not possible.
#2 lore raw
Quoted 28 lines · Marc Zyngier
Quoted 26 lines · Greg Kroah-Hartman

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.

---
 kernel/irq/irqdomain.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
kernel/irq/irqdomain.c +1 -1
index 8fe1da9614ee..e2096b51c004 100644
@@ -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);
 }
 
Quoted 5 lines

kernel/irq/irqdomain.c static void debugfs_add_domain_dir(struct irq_domain *d) · line 1921

Quoted 1 line
kernel/irq/irqdomain.c +0 -0
 void __init irq_domain_debugfs_init(struct dentry *root)

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

#3 lore raw
Quoted 30 lines · Greg Kroah-Hartman
Quoted 28 lines · Marc Zyngier
Quoted 26 lines · Greg Kroah-Hartman

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.

---
 kernel/irq/irqdomain.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
kernel/irq/irqdomain.c +1 -1
index 8fe1da9614ee..e2096b51c004 100644
@@ -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);
 }
 
Quoted 8 lines
Quoted 5 lines

kernel/irq/irqdomain.c static void debugfs_add_domain_dir(struct irq_domain *d) · line 1921

Quoted 1 line
kernel/irq/irqdomain.c +0 -0
 void __init irq_domain_debugfs_init(struct dentry *root)

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).

Quoted 2 lines

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.
Signature · 1 line
Without deviation from the norm, progress is not possible.
#4 lore raw
Quoted 36 lines · Marc Zyngier
Quoted 34 lines · Greg Kroah-Hartman
Quoted 32 lines · Marc Zyngier
Quoted 19 lines · Greg Kroah-Hartman

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.

---
 kernel/irq/irqdomain.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
kernel/irq/irqdomain.c +0 -0
index 8fe1da9614ee..e2096b51c004 100644

kernel/irq/irqdomain.c static void debugfs_add_domain_dir(struct irq_domain *d) · line 1921

Quoted 8 lines
kernel/irq/irqdomain.c +1 -1
@@ -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)
Quoted 8 lines
Quoted 4 lines
Quoted 1 line

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