[PATCH] pm/opp: fix race between opp_add and opp_get

Xuewen Yan posted 1 patch 1 month, 2 weeks ago
drivers/opp/core.c | 5 ++---
1 file changed, 2 insertions(+), 3 deletions(-)
[PATCH] pm/opp: fix race between opp_add and opp_get
Posted by Xuewen Yan 1 month, 2 weeks ago
From: Di Shen <di.shen@unisoc.com>

There is a race between opp_table_find_key and dev_pm_opp_add_dynamic:

dev_pm_opp_add_dynamic                dev_pm_opp_find_freq_exact
_opp_add_v1                           _find_key
_opp_add                              _opp_table_find_key
mutex_lock(opp_table->lock)
list_add(&new_opp->node, head)
mutex_unlock(opp_table->lock)
                                     mutex_lock(opp_table->lock)
                                     dev_pm_opp_get(opp)
                                     kref_get(&opp->kref)
                                       refcount_inc(&kref->refcount);
                                     mutex_unlock(opp_table->lock)
kref_init(&new_opp->kref)
  refcount_set(&kref->refcount, 1);
                                     dev_pm_opp_put()
                                     kref_put_mutex()

This would cause the opp be freed.

So use the mutex to protect opp's ref count.

Co-developed-by: Ling Xu <ling_ling.xu@unisoc.com>
Signed-off-by: Ling Xu <ling_ling.xu@unisoc.com>
Signed-off-by: Di Shen <di.shen@unisoc.com>
---
 drivers/opp/core.c | 5 ++---
 1 file changed, 2 insertions(+), 3 deletions(-)

diff --git a/drivers/opp/core.c b/drivers/opp/core.c
index 866641666e41..34cdcf8f598d 100644
--- a/drivers/opp/core.c
+++ b/drivers/opp/core.c
@@ -2088,11 +2088,10 @@ int _opp_add(struct device *dev, struct dev_pm_opp *new_opp,
 			return ret;
 
 		list_add(&new_opp->node, head);
+		new_opp->opp_table = opp_table;
+		kref_init(&new_opp->kref);
 	}
 
-	new_opp->opp_table = opp_table;
-	kref_init(&new_opp->kref);
-
 	opp_debug_create_one(new_opp, opp_table);
 
 	if (!_opp_supported_by_regulators(new_opp, opp_table)) {
-- 
2.25.1
Re: [PATCH] pm/opp: fix race between opp_add and opp_get
Posted by Viresh Kumar 1 month, 1 week ago
On 27-04-26, 20:00, Xuewen Yan wrote:
> From: Di Shen <di.shen@unisoc.com>
> 
> There is a race between opp_table_find_key and dev_pm_opp_add_dynamic:
> 
> dev_pm_opp_add_dynamic                dev_pm_opp_find_freq_exact
> _opp_add_v1                           _find_key
> _opp_add                              _opp_table_find_key
> mutex_lock(opp_table->lock)
> list_add(&new_opp->node, head)
> mutex_unlock(opp_table->lock)
>                                      mutex_lock(opp_table->lock)
>                                      dev_pm_opp_get(opp)
>                                      kref_get(&opp->kref)
>                                        refcount_inc(&kref->refcount);
>                                      mutex_unlock(opp_table->lock)
> kref_init(&new_opp->kref)
>   refcount_set(&kref->refcount, 1);
>                                      dev_pm_opp_put()
>                                      kref_put_mutex()
> 
> This would cause the opp be freed.
> 
> So use the mutex to protect opp's ref count.
> 
> Co-developed-by: Ling Xu <ling_ling.xu@unisoc.com>
> Signed-off-by: Ling Xu <ling_ling.xu@unisoc.com>
> Signed-off-by: Di Shen <di.shen@unisoc.com>
> ---
>  drivers/opp/core.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)

Good catch.

Applied. Thanks.

-- 
viresh
Re: [PATCH] pm/opp: fix race between opp_add and opp_get
Posted by Xuewen Yan 1 month, 1 week ago
Hi Viresh,

On Tue, May 5, 2026 at 11:36 AM Viresh Kumar <viresh.kumar@linaro.org> wrote:
>
> On 27-04-26, 20:00, Xuewen Yan wrote:
> > From: Di Shen <di.shen@unisoc.com>
> >
> > There is a race between opp_table_find_key and dev_pm_opp_add_dynamic:
> >
> > dev_pm_opp_add_dynamic                dev_pm_opp_find_freq_exact
> > _opp_add_v1                           _find_key
> > _opp_add                              _opp_table_find_key
> > mutex_lock(opp_table->lock)
> > list_add(&new_opp->node, head)
> > mutex_unlock(opp_table->lock)
> >                                      mutex_lock(opp_table->lock)
> >                                      dev_pm_opp_get(opp)
> >                                      kref_get(&opp->kref)
> >                                        refcount_inc(&kref->refcount);
> >                                      mutex_unlock(opp_table->lock)
> > kref_init(&new_opp->kref)
> >   refcount_set(&kref->refcount, 1);
> >                                      dev_pm_opp_put()
> >                                      kref_put_mutex()
> >
> > This would cause the opp be freed.
> >
> > So use the mutex to protect opp's ref count.
> >
> > Co-developed-by: Ling Xu <ling_ling.xu@unisoc.com>
> > Signed-off-by: Ling Xu <ling_ling.xu@unisoc.com>
> > Signed-off-by: Di Shen <di.shen@unisoc.com>
> > ---
> >  drivers/opp/core.c | 5 ++---
> >  1 file changed, 2 insertions(+), 3 deletions(-)
>
> Good catch.
>
> Applied. Thanks.

Need we add fix tag?
Fixes: 7034764a1e4a (PM / OPP: Add 'struct kref' to struct dev_pm_opp)

>
> --
> viresh
Re: [PATCH] pm/opp: fix race between opp_add and opp_get
Posted by Viresh Kumar 1 month, 1 week ago
On 07-05-26, 10:09, Xuewen Yan wrote:
> Need we add fix tag?
> Fixes: 7034764a1e4a (PM / OPP: Add 'struct kref' to struct dev_pm_opp)

Done

-- 
viresh
Re: [PATCH] pm/opp: fix race between opp_add and opp_get
Posted by Xuewen Yan 1 month, 2 weeks ago
Hi Viresh,

Do you have any comments on this patch?

Thanks!

On Mon, Apr 27, 2026 at 8:01 PM Xuewen Yan <xuewen.yan@unisoc.com> wrote:
>
> From: Di Shen <di.shen@unisoc.com>
>
> There is a race between opp_table_find_key and dev_pm_opp_add_dynamic:
>
> dev_pm_opp_add_dynamic                dev_pm_opp_find_freq_exact
> _opp_add_v1                           _find_key
> _opp_add                              _opp_table_find_key
> mutex_lock(opp_table->lock)
> list_add(&new_opp->node, head)
> mutex_unlock(opp_table->lock)
>                                      mutex_lock(opp_table->lock)
>                                      dev_pm_opp_get(opp)
>                                      kref_get(&opp->kref)
>                                        refcount_inc(&kref->refcount);
>                                      mutex_unlock(opp_table->lock)
> kref_init(&new_opp->kref)
>   refcount_set(&kref->refcount, 1);
>                                      dev_pm_opp_put()
>                                      kref_put_mutex()
>
> This would cause the opp be freed.
>
> So use the mutex to protect opp's ref count.
>
> Co-developed-by: Ling Xu <ling_ling.xu@unisoc.com>
> Signed-off-by: Ling Xu <ling_ling.xu@unisoc.com>
> Signed-off-by: Di Shen <di.shen@unisoc.com>
> ---
>  drivers/opp/core.c | 5 ++---
>  1 file changed, 2 insertions(+), 3 deletions(-)
>
> diff --git a/drivers/opp/core.c b/drivers/opp/core.c
> index 866641666e41..34cdcf8f598d 100644
> --- a/drivers/opp/core.c
> +++ b/drivers/opp/core.c
> @@ -2088,11 +2088,10 @@ int _opp_add(struct device *dev, struct dev_pm_opp *new_opp,
>                         return ret;
>
>                 list_add(&new_opp->node, head);
> +               new_opp->opp_table = opp_table;
> +               kref_init(&new_opp->kref);
>         }
>
> -       new_opp->opp_table = opp_table;
> -       kref_init(&new_opp->kref);
> -
>         opp_debug_create_one(new_opp, opp_table);
>
>         if (!_opp_supported_by_regulators(new_opp, opp_table)) {
> --
> 2.25.1
>