1. Patchew
  2. linux
  3. View series

[PATCH] omap-cpufreq: Fix regulator resource leak in probe()

Haotian Zhang posted 1 patch 1 month, 3 weeks ago 
drivers/cpufreq/omap-cpufreq.c | 3 +--
1 file changed, 1 insertion(+), 2 deletions(-)
[PATCH] omap-cpufreq: Fix regulator resource leak in probe()
Posted by Haotian Zhang 1 month, 3 weeks ago 
The current omap_cpufreq_probe() uses regulator_get() to obtain the MPU
regulator but does not release it in omap_cpufreq_remove() or when
cpufreq_register_driver() fails, leading to a potential resource leak.

Use devm_regulator_get() instead of regulator_get() so that the regulator
resource is automatically released.

Fixes: 53dfe8a884e6 ("cpufreq: OMAP: scale voltage along with frequency")
Signed-off-by: Haotian Zhang <vulab@iscas.ac.cn>
---
 drivers/cpufreq/omap-cpufreq.c | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/drivers/cpufreq/omap-cpufreq.c b/drivers/cpufreq/omap-cpufreq.c
index bbb01d93b54b..f83f85996b36 100644
--- a/drivers/cpufreq/omap-cpufreq.c
+++ b/drivers/cpufreq/omap-cpufreq.c
@@ -157,7 +157,7 @@ static int omap_cpufreq_probe(struct platform_device *pdev)
 		return -EINVAL;
 	}
 
-	mpu_reg = regulator_get(mpu_dev, "vcc");
+	mpu_reg = devm_regulator_get(mpu_dev, "vcc");
 	if (IS_ERR(mpu_reg)) {
 		pr_warn("%s: unable to get MPU regulator\n", __func__);
 		mpu_reg = NULL;
@@ -169,7 +169,6 @@ static int omap_cpufreq_probe(struct platform_device *pdev)
 		if (regulator_get_voltage(mpu_reg) < 0) {
 			pr_warn("%s: physical regulator not present for MPU\n",
 				__func__);
-			regulator_put(mpu_reg);
 			mpu_reg = NULL;
 		}
 	}
-- 
2.50.1.windows.1
Re: [PATCH] omap-cpufreq: Fix regulator resource leak in probe()
Posted by Andreas Kemnade 1 month ago 
On Mon, 15 Dec 2025 11:03:27 +0800
Haotian Zhang <vulab@iscas.ac.cn> wrote:

> The current omap_cpufreq_probe() uses regulator_get() to obtain the MPU
> regulator but does not release it in omap_cpufreq_remove() or when
> cpufreq_register_driver() fails, leading to a potential resource leak.
> 
> Use devm_regulator_get() instead of regulator_get() so that the regulator
> resource is automatically released.
> 
> Fixes: 53dfe8a884e6 ("cpufreq: OMAP: scale voltage along with frequency")
> Signed-off-by: Haotian Zhang <vulab@iscas.ac.cn>
> ---
>  drivers/cpufreq/omap-cpufreq.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/cpufreq/omap-cpufreq.c b/drivers/cpufreq/omap-cpufreq.c
> index bbb01d93b54b..f83f85996b36 100644
> --- a/drivers/cpufreq/omap-cpufreq.c
> +++ b/drivers/cpufreq/omap-cpufreq.c
> @@ -157,7 +157,7 @@ static int omap_cpufreq_probe(struct platform_device *pdev)
>  		return -EINVAL;
>  	}
>  
> -	mpu_reg = regulator_get(mpu_dev, "vcc");
> +	mpu_reg = devm_regulator_get(mpu_dev, "vcc");
>  	if (IS_ERR(mpu_reg)) {
>  		pr_warn("%s: unable to get MPU regulator\n", __func__);
>  		mpu_reg = NULL;
> @@ -169,7 +169,6 @@ static int omap_cpufreq_probe(struct platform_device *pdev)
>  		if (regulator_get_voltage(mpu_reg) < 0) {
>  			pr_warn("%s: physical regulator not present for MPU\n",
>  				__func__);
> -			regulator_put(mpu_reg);

so it it not useable and could be released which is not done anymare 
with your patch. It is not an error path here.

>  			mpu_reg = NULL;

And this should happen after removal, too. I feel some discomfort with
variables pointing to freed ressources. So I think rather add
the regulator_put and the = NULL to the remove function.

Regards,
Andreas
Re: [PATCH] omap-cpufreq: Fix regulator resource leak in probe()
Posted by Viresh Kumar 1 month ago 
On 05-01-26, 10:14, Andreas Kemnade wrote:
> On Mon, 15 Dec 2025 11:03:27 +0800
> Haotian Zhang <vulab@iscas.ac.cn> wrote:
> 
> > The current omap_cpufreq_probe() uses regulator_get() to obtain the MPU
> > regulator but does not release it in omap_cpufreq_remove() or when
> > cpufreq_register_driver() fails, leading to a potential resource leak.
> > 
> > Use devm_regulator_get() instead of regulator_get() so that the regulator
> > resource is automatically released.
> > 
> > Fixes: 53dfe8a884e6 ("cpufreq: OMAP: scale voltage along with frequency")
> > Signed-off-by: Haotian Zhang <vulab@iscas.ac.cn>
> > ---
> >  drivers/cpufreq/omap-cpufreq.c | 3 +--
> >  1 file changed, 1 insertion(+), 2 deletions(-)
> > 
> > diff --git a/drivers/cpufreq/omap-cpufreq.c b/drivers/cpufreq/omap-cpufreq.c
> > index bbb01d93b54b..f83f85996b36 100644
> > --- a/drivers/cpufreq/omap-cpufreq.c
> > +++ b/drivers/cpufreq/omap-cpufreq.c
> > @@ -157,7 +157,7 @@ static int omap_cpufreq_probe(struct platform_device *pdev)
> >  		return -EINVAL;
> >  	}
> >  
> > -	mpu_reg = regulator_get(mpu_dev, "vcc");
> > +	mpu_reg = devm_regulator_get(mpu_dev, "vcc");
> >  	if (IS_ERR(mpu_reg)) {
> >  		pr_warn("%s: unable to get MPU regulator\n", __func__);
> >  		mpu_reg = NULL;
> > @@ -169,7 +169,6 @@ static int omap_cpufreq_probe(struct platform_device *pdev)
> >  		if (regulator_get_voltage(mpu_reg) < 0) {
> >  			pr_warn("%s: physical regulator not present for MPU\n",
> >  				__func__);
> > -			regulator_put(mpu_reg);
> 
> so it it not useable and could be released which is not done anymare 
> with your patch. It is not an error path here.

Right. Perhaps devm_regulator_put() here would be good enough.

> >  			mpu_reg = NULL;
> 
> And this should happen after removal, too. I feel some discomfort with
> variables pointing to freed ressources. So I think rather add
> the regulator_put and the = NULL to the remove function.

I don't see a reason why this extra step should be performed after the driver is
removed. `mpu_reg` can't be used after that.

-- 
viresh
Re: [PATCH] omap-cpufreq: Fix regulator resource leak in probe()
Posted by Andreas Kemnade 1 month ago 
On Tue, 6 Jan 2026 10:20:55 +0530
Viresh Kumar <viresh.kumar@linaro.org> wrote:

> On 05-01-26, 10:14, Andreas Kemnade wrote:
> > On Mon, 15 Dec 2025 11:03:27 +0800
> > Haotian Zhang <vulab@iscas.ac.cn> wrote:
> >   
> > > The current omap_cpufreq_probe() uses regulator_get() to obtain the MPU
> > > regulator but does not release it in omap_cpufreq_remove() or when
> > > cpufreq_register_driver() fails, leading to a potential resource leak.
> > > 
> > > Use devm_regulator_get() instead of regulator_get() so that the regulator
> > > resource is automatically released.
> > > 
> > > Fixes: 53dfe8a884e6 ("cpufreq: OMAP: scale voltage along with frequency")
> > > Signed-off-by: Haotian Zhang <vulab@iscas.ac.cn>
> > > ---
> > >  drivers/cpufreq/omap-cpufreq.c | 3 +--
> > >  1 file changed, 1 insertion(+), 2 deletions(-)
> > > 
> > > diff --git a/drivers/cpufreq/omap-cpufreq.c b/drivers/cpufreq/omap-cpufreq.c
> > > index bbb01d93b54b..f83f85996b36 100644
> > > --- a/drivers/cpufreq/omap-cpufreq.c
> > > +++ b/drivers/cpufreq/omap-cpufreq.c
> > > @@ -157,7 +157,7 @@ static int omap_cpufreq_probe(struct platform_device *pdev)
> > >  		return -EINVAL;
> > >  	}
> > >  
> > > -	mpu_reg = regulator_get(mpu_dev, "vcc");
> > > +	mpu_reg = devm_regulator_get(mpu_dev, "vcc");
> > >  	if (IS_ERR(mpu_reg)) {
> > >  		pr_warn("%s: unable to get MPU regulator\n", __func__);
> > >  		mpu_reg = NULL;
> > > @@ -169,7 +169,6 @@ static int omap_cpufreq_probe(struct platform_device *pdev)
> > >  		if (regulator_get_voltage(mpu_reg) < 0) {
> > >  			pr_warn("%s: physical regulator not present for MPU\n",
> > >  				__func__);
> > > -			regulator_put(mpu_reg);  
> > 
> > so it it not useable and could be released which is not done anymare 
> > with your patch. It is not an error path here.  
> 
> Right. Perhaps devm_regulator_put() here would be good enough.
> 
ok, didn't expect such a function, so that should be the cleanest approach.

> > >  			mpu_reg = NULL;  
> > 
> > And this should happen after removal, too. I feel some discomfort with
> > variables pointing to freed ressources. So I think rather add
> > the regulator_put and the = NULL to the remove function.  
> 
> I don't see a reason why this extra step should be performed after the driver is
> removed. `mpu_reg` can't be used after that.
> 
hmm, it is performed when the device is removed/unbound, which does not necessarily
mean the driver is removed. But that does not prevent trouble if something
is still trying to access stuff here after driver removal. So it is not really
helpful.

Hmm, how does a device gets bound to this driver?

Lets gets back to this very basic question. I am usually using CPUFREQ_DT.
Are there any signs of usage of this driver?

omap2plus_defconfig creates in .config
#
# CPU frequency scaling drivers
#
CONFIG_CPUFREQ_DT=m
# CONFIG_CPUFREQ_VIRT is not set
CONFIG_CPUFREQ_DT_PLATDEV=y
# CONFIG_ARM_OMAP2PLUS_CPUFREQ is not set
CONFIG_ARM_TI_CPUFREQ=y
# end of CPU Frequency scaling

So this thing is not used. Everything with omap2plus uses devicetree,
so probably no user at all for it. So I think we can deorbit the whole
thing.

But the fix is good for stable. So I would propose to add this
fix (to let it propagate to stable) and deorbit this driver.

Regards,
Andreas
Re: [PATCH] omap-cpufreq: Fix regulator resource leak in probe()
Posted by Viresh Kumar 1 month ago 
On 06-01-26, 18:29, Andreas Kemnade wrote:
> hmm, it is performed when the device is removed/unbound, which does not necessarily
> mean the driver is removed.

For the cpufreq drivers, the device is normally never removed. It either gets
created from DT or some platform specific code creates the device for ever. But
anyway, we were both talking about unbound being called, doesn't matter if it is
the device or driver which is removed.

> But that does not prevent trouble if something
> is still trying to access stuff here after driver removal. So it is not really
> helpful.

It is not possible for something to still be using the resources from this
driver (like the global variables) after remove() is called. If there is a bug
in there, then that needs to be fixed instead.

> Hmm, how does a device gets bound to this driver?

Nice catch.

Tried to look at history.

commit cb6675d6a868 ("ARM: OMAP2+: Remove legacy PM init")

This commit removed the platform device being created and mentions that stuff
happens via DT, which AFAIU, creates the cpufreq-dt device instead.

So no one should be using this driver since year 2016.

Kevin, Nishanth, can you please confirm ? We should remove this driver.

> But the fix is good for stable. So I would propose to add this
> fix (to let it propagate to stable) and deorbit this driver.

I don't think it is worth adding to stable when there are no users.

-- 
viresh
Re: [PATCH] omap-cpufreq: Fix regulator resource leak in probe()
Posted by Andreas Kemnade 1 month ago 
On Wed, 7 Jan 2026 11:27:42 +0530
Viresh Kumar <viresh.kumar@linaro.org> wrote:

> > But the fix is good for stable. So I would propose to add this
> > fix (to let it propagate to stable) and deorbit this driver.  
> 
> I don't think it is worth adding to stable when there are no users.

ok, thet commit is from v4.9, oldest longterm is 5.10.
So also no users in longterm kernels, so we do not need the fix at all.

Regards,
Andreas
Re: [PATCH] omap-cpufreq: Fix regulator resource leak in probe()
Posted by Viresh Kumar 1 month ago 
On 15-12-25, 11:03, Haotian Zhang wrote:
> The current omap_cpufreq_probe() uses regulator_get() to obtain the MPU
> regulator but does not release it in omap_cpufreq_remove() or when
> cpufreq_register_driver() fails, leading to a potential resource leak.
> 
> Use devm_regulator_get() instead of regulator_get() so that the regulator
> resource is automatically released.
> 
> Fixes: 53dfe8a884e6 ("cpufreq: OMAP: scale voltage along with frequency")
> Signed-off-by: Haotian Zhang <vulab@iscas.ac.cn>
> ---
>  drivers/cpufreq/omap-cpufreq.c | 3 +--
>  1 file changed, 1 insertion(+), 2 deletions(-)
> 
> diff --git a/drivers/cpufreq/omap-cpufreq.c b/drivers/cpufreq/omap-cpufreq.c
> index bbb01d93b54b..f83f85996b36 100644
> --- a/drivers/cpufreq/omap-cpufreq.c
> +++ b/drivers/cpufreq/omap-cpufreq.c
> @@ -157,7 +157,7 @@ static int omap_cpufreq_probe(struct platform_device *pdev)
>  		return -EINVAL;
>  	}
>  
> -	mpu_reg = regulator_get(mpu_dev, "vcc");
> +	mpu_reg = devm_regulator_get(mpu_dev, "vcc");
>  	if (IS_ERR(mpu_reg)) {
>  		pr_warn("%s: unable to get MPU regulator\n", __func__);
>  		mpu_reg = NULL;
> @@ -169,7 +169,6 @@ static int omap_cpufreq_probe(struct platform_device *pdev)
>  		if (regulator_get_voltage(mpu_reg) < 0) {
>  			pr_warn("%s: physical regulator not present for MPU\n",
>  				__func__);
> -			regulator_put(mpu_reg);
>  			mpu_reg = NULL;
>  		}
>  	}

Kevin ?

-- 
viresh
[PATCH v2] cpufreq: OMAP: Fix resource leak in probe error path and remove
Posted by Haotian Zhang 1 month ago 
The current omap_cpufreq_probe() uses regulator_get() to obtain the MPU
regulator but does not release it in omap_cpufreq_remove() or when
cpufreq_register_driver() fails.

Add the missing regulator_put() in the remove function and in the
error handling path of the probe function to prevent resource leaks.
Also ensure the mpu_reg pointer is set to NULL after release to avoid
dangling pointers.

Fixes: 53dfe8a884e6 ("cpufreq: OMAP: scale voltage along with frequency")
Suggested-by: Andreas Kemnade <andreas@kemnade.info>
Signed-off-by: Haotian Zhang <vulab@iscas.ac.cn>

---
Changes in v2:
 - Revert to using regulator_get() instead of devm_regulator_get()
   to ensure immediate release of unusable regulators and
   safer handling of the global mpu_reg variable.
 - Add explicit regulator_put() in omap_cpufreq_remove().
 - Add error handling for cpufreq_register_driver() in probe.
---
 drivers/cpufreq/omap-cpufreq.c | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/drivers/cpufreq/omap-cpufreq.c b/drivers/cpufreq/omap-cpufreq.c
index bbb01d93b54b..b3d58090d202 100644
--- a/drivers/cpufreq/omap-cpufreq.c
+++ b/drivers/cpufreq/omap-cpufreq.c
@@ -151,6 +151,8 @@ static struct cpufreq_driver omap_driver = {
 
 static int omap_cpufreq_probe(struct platform_device *pdev)
 {
+	int ret;
+
 	mpu_dev = get_cpu_device(0);
 	if (!mpu_dev) {
 		pr_warn("%s: unable to get the MPU device\n", __func__);
@@ -174,12 +176,23 @@ static int omap_cpufreq_probe(struct platform_device *pdev)
 		}
 	}
 
-	return cpufreq_register_driver(&omap_driver);
+	ret = cpufreq_register_driver(&omap_driver);
+	if (ret) {
+		if (mpu_reg) {
+			regulator_put(mpu_reg);
+			mpu_reg = NULL;
+		}
+	}
+	return ret;
 }
 
 static void omap_cpufreq_remove(struct platform_device *pdev)
 {
 	cpufreq_unregister_driver(&omap_driver);
+	if (mpu_reg) {
+		regulator_put(mpu_reg);
+		mpu_reg = NULL;
+	}
 }
 
 static struct platform_driver omap_cpufreq_platdrv = {
-- 
2.43.0
Re: [PATCH v2] cpufreq: OMAP: Fix resource leak in probe error path and remove
Posted by Viresh Kumar 1 month ago 
On 05-01-26, 21:12, Haotian Zhang wrote:
> The current omap_cpufreq_probe() uses regulator_get() to obtain the MPU
> regulator but does not release it in omap_cpufreq_remove() or when
> cpufreq_register_driver() fails.
> 
> Add the missing regulator_put() in the remove function and in the
> error handling path of the probe function to prevent resource leaks.
> Also ensure the mpu_reg pointer is set to NULL after release to avoid
> dangling pointers.
> 
> Fixes: 53dfe8a884e6 ("cpufreq: OMAP: scale voltage along with frequency")
> Suggested-by: Andreas Kemnade <andreas@kemnade.info>
> Signed-off-by: Haotian Zhang <vulab@iscas.ac.cn>
> 
> ---
> Changes in v2:
>  - Revert to using regulator_get() instead of devm_regulator_get()
>    to ensure immediate release of unusable regulators and
>    safer handling of the global mpu_reg variable.
>  - Add explicit regulator_put() in omap_cpufreq_remove().
>  - Add error handling for cpufreq_register_driver() in probe.
> ---
>  drivers/cpufreq/omap-cpufreq.c | 15 ++++++++++++++-
>  1 file changed, 14 insertions(+), 1 deletion(-)

I have left some feedback on the previous version. Lets continue the discussion
there.

-- 
viresh

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.