| Message ID | 20230202141545.2295679-1-gregkh@linuxfoundation.org (mailing list archive) |
|---|---|
| State | Mainlined, archived |
| Headers | show |
| Series | drivers: base: power: fix memory leak with using debugfs_lookup() | expand |
On Thu, Feb 2, 2023 at 3:15 PM 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: "Rafael J. Wysocki" <rafael@kernel.org> > Cc: Kevin Hilman <khilman@kernel.org> > Cc: Ulf Hansson <ulf.hansson@linaro.org> > Cc: Pavel Machek <pavel@ucw.cz> > Cc: Len Brown <len.brown@intel.com> > Cc: linux-pm@vger.kernel.org > Cc: linux-kernel@vger.kernel.org > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > --- > drivers/base/power/domain.c | 5 +---- > 1 file changed, 1 insertion(+), 4 deletions(-) > > diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c > index 967bcf9d415e..6097644ebdc5 100644 > --- a/drivers/base/power/domain.c > +++ b/drivers/base/power/domain.c > @@ -220,13 +220,10 @@ static void genpd_debug_add(struct generic_pm_domain *genpd); > > static void genpd_debug_remove(struct generic_pm_domain *genpd) > { > - struct dentry *d; > - > if (!genpd_debugfs_dir) > return; > > - d = debugfs_lookup(genpd->name, genpd_debugfs_dir); > - debugfs_remove(d); > + debugfs_lookup_and_remove(genpd->name, genpd_debugfs_dir); > } > > static void genpd_update_accounting(struct generic_pm_domain *genpd) > -- Does this depend on anything in your tree, or can I apply it?
On Thu, 2 Feb 2023 at 15:15, 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: "Rafael J. Wysocki" <rafael@kernel.org> > Cc: Kevin Hilman <khilman@kernel.org> > Cc: Ulf Hansson <ulf.hansson@linaro.org> > Cc: Pavel Machek <pavel@ucw.cz> > Cc: Len Brown <len.brown@intel.com> > Cc: linux-pm@vger.kernel.org > Cc: linux-kernel@vger.kernel.org > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> Reviewed-by: Ulf Hansson <ulf.hansson@linaro.org> Kind regards Uffe > --- > drivers/base/power/domain.c | 5 +---- > 1 file changed, 1 insertion(+), 4 deletions(-) > > diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c > index 967bcf9d415e..6097644ebdc5 100644 > --- a/drivers/base/power/domain.c > +++ b/drivers/base/power/domain.c > @@ -220,13 +220,10 @@ static void genpd_debug_add(struct generic_pm_domain *genpd); > > static void genpd_debug_remove(struct generic_pm_domain *genpd) > { > - struct dentry *d; > - > if (!genpd_debugfs_dir) > return; > > - d = debugfs_lookup(genpd->name, genpd_debugfs_dir); > - debugfs_remove(d); > + debugfs_lookup_and_remove(genpd->name, genpd_debugfs_dir); > } > > static void genpd_update_accounting(struct generic_pm_domain *genpd) > -- > 2.39.1 >
On Thu, Feb 02, 2023 at 03:23:46PM +0100, Rafael J. Wysocki wrote: > On Thu, Feb 2, 2023 at 3:15 PM 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: "Rafael J. Wysocki" <rafael@kernel.org> > > Cc: Kevin Hilman <khilman@kernel.org> > > Cc: Ulf Hansson <ulf.hansson@linaro.org> > > Cc: Pavel Machek <pavel@ucw.cz> > > Cc: Len Brown <len.brown@intel.com> > > Cc: linux-pm@vger.kernel.org > > Cc: linux-kernel@vger.kernel.org > > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > > --- > > drivers/base/power/domain.c | 5 +---- > > 1 file changed, 1 insertion(+), 4 deletions(-) > > > > diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c > > index 967bcf9d415e..6097644ebdc5 100644 > > --- a/drivers/base/power/domain.c > > +++ b/drivers/base/power/domain.c > > @@ -220,13 +220,10 @@ static void genpd_debug_add(struct generic_pm_domain *genpd); > > > > static void genpd_debug_remove(struct generic_pm_domain *genpd) > > { > > - struct dentry *d; > > - > > if (!genpd_debugfs_dir) > > return; > > > > - d = debugfs_lookup(genpd->name, genpd_debugfs_dir); > > - debugfs_remove(d); > > + debugfs_lookup_and_remove(genpd->name, genpd_debugfs_dir); > > } > > > > static void genpd_update_accounting(struct generic_pm_domain *genpd) > > -- > > Does this depend on anything in your tree, or can I apply it? It does not depend on anything in any of my trees, it's made against Linus's tree right now so please take it through yours. thanks! greg k-h
On Thu, Feb 2, 2023 at 4:09 PM Greg Kroah-Hartman <gregkh@linuxfoundation.org> wrote: > > On Thu, Feb 02, 2023 at 03:23:46PM +0100, Rafael J. Wysocki wrote: > > On Thu, Feb 2, 2023 at 3:15 PM 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: "Rafael J. Wysocki" <rafael@kernel.org> > > > Cc: Kevin Hilman <khilman@kernel.org> > > > Cc: Ulf Hansson <ulf.hansson@linaro.org> > > > Cc: Pavel Machek <pavel@ucw.cz> > > > Cc: Len Brown <len.brown@intel.com> > > > Cc: linux-pm@vger.kernel.org > > > Cc: linux-kernel@vger.kernel.org > > > Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> > > > --- > > > drivers/base/power/domain.c | 5 +---- > > > 1 file changed, 1 insertion(+), 4 deletions(-) > > > > > > diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c > > > index 967bcf9d415e..6097644ebdc5 100644 > > > --- a/drivers/base/power/domain.c > > > +++ b/drivers/base/power/domain.c > > > @@ -220,13 +220,10 @@ static void genpd_debug_add(struct generic_pm_domain *genpd); > > > > > > static void genpd_debug_remove(struct generic_pm_domain *genpd) > > > { > > > - struct dentry *d; > > > - > > > if (!genpd_debugfs_dir) > > > return; > > > > > > - d = debugfs_lookup(genpd->name, genpd_debugfs_dir); > > > - debugfs_remove(d); > > > + debugfs_lookup_and_remove(genpd->name, genpd_debugfs_dir); > > > } > > > > > > static void genpd_update_accounting(struct generic_pm_domain *genpd) > > > -- > > > > Does this depend on anything in your tree, or can I apply it? > > It does not depend on anything in any of my trees, it's made against > Linus's tree right now so please take it through yours. Applied for 6.3 now, thanks!
diff --git a/drivers/base/power/domain.c b/drivers/base/power/domain.c index 967bcf9d415e..6097644ebdc5 100644 --- a/drivers/base/power/domain.c +++ b/drivers/base/power/domain.c @@ -220,13 +220,10 @@ static void genpd_debug_add(struct generic_pm_domain *genpd); static void genpd_debug_remove(struct generic_pm_domain *genpd) { - struct dentry *d; - if (!genpd_debugfs_dir) return; - d = debugfs_lookup(genpd->name, genpd_debugfs_dir); - debugfs_remove(d); + debugfs_lookup_and_remove(genpd->name, genpd_debugfs_dir); } static void genpd_update_accounting(struct generic_pm_domain *genpd)
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: "Rafael J. Wysocki" <rafael@kernel.org> Cc: Kevin Hilman <khilman@kernel.org> Cc: Ulf Hansson <ulf.hansson@linaro.org> Cc: Pavel Machek <pavel@ucw.cz> Cc: Len Brown <len.brown@intel.com> Cc: linux-pm@vger.kernel.org Cc: linux-kernel@vger.kernel.org Signed-off-by: Greg Kroah-Hartman <gregkh@linuxfoundation.org> --- drivers/base/power/domain.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-)