Messages in this thread | | | Date | Fri, 25 Mar 2022 09:01:55 +0000 | Subject | Re: [PATCH v2] thermal: devfreq_cooling: use local ops instead of global ops | From | Lukasz Luba <> |
| |
Hi Kant,
On 3/25/22 07:30, Kant Fan wrote: > Fix access illegal address problem in following condition: > There are muti devfreq cooling devices in system, some of them has > em model but other does not, energy model ops such as state2power will > append to global devfreq_cooling_ops when the cooling device with > em model register. It makes the cooling device without em model > also use devfreq_cooling_ops after appending when register later by > of_devfreq_cooling_register_power() or of_devfreq_cooling_register(). > > IPA governor regards the cooling devices without em model as a power actor > because they also have energy model ops, and will access illegal address > at dfc->em_pd when execute cdev->ops->get_requested_power, > cdev->ops->state2power or cdev->ops->power2state. > > Fixes: 615510fe13bd2 ("thermal: devfreq_cooling: remove old power model and use EM") > Cc: stable@vger.kernel.org # 5.13+ > Signed-off-by: Kant Fan <kant@allwinnertech.com> > --- > drivers/thermal/devfreq_cooling.c | 25 ++++++++++++++++++------- > 1 file changed, 18 insertions(+), 7 deletions(-) > > diff --git a/drivers/thermal/devfreq_cooling.c b/drivers/thermal/devfreq_cooling.c > index 4310cb342a9f..d38a80adec73 100644 > --- a/drivers/thermal/devfreq_cooling.c > +++ b/drivers/thermal/devfreq_cooling.c > @@ -358,21 +358,28 @@ of_devfreq_cooling_register_power(struct device_node *np, struct devfreq *df, > struct thermal_cooling_device *cdev; > struct device *dev = df->dev.parent; > struct devfreq_cooling_device *dfc; > + struct thermal_cooling_device_ops *ops; > char *name; > int err, num_opps; > > - dfc = kzalloc(sizeof(*dfc), GFP_KERNEL); > - if (!dfc) > + ops = kmemdup(&devfreq_cooling_ops, sizeof(*ops), GFP_KERNEL); > + if (!ops) > return ERR_PTR(-ENOMEM); > > + dfc = kzalloc(sizeof(*dfc), GFP_KERNEL); > + if (!dfc) { > + err = -ENOMEM; > + goto free_ops; > + } > + > dfc->devfreq = df; > > dfc->em_pd = em_pd_get(dev); > if (dfc->em_pd) { > - devfreq_cooling_ops.get_requested_power = > + ops->get_requested_power = > devfreq_cooling_get_requested_power; > - devfreq_cooling_ops.state2power = devfreq_cooling_state2power; > - devfreq_cooling_ops.power2state = devfreq_cooling_power2state; > + ops->state2power = devfreq_cooling_state2power; > + ops->power2state = devfreq_cooling_power2state; > > dfc->power_ops = dfc_power; > > @@ -407,8 +414,7 @@ of_devfreq_cooling_register_power(struct device_node *np, struct devfreq *df, > if (!name) > goto remove_qos_req; > > - cdev = thermal_of_cooling_device_register(np, name, dfc, > - &devfreq_cooling_ops); > + cdev = thermal_of_cooling_device_register(np, name, dfc, ops); > kfree(name); > > if (IS_ERR(cdev)) { > @@ -429,6 +435,8 @@ of_devfreq_cooling_register_power(struct device_node *np, struct devfreq *df, > kfree(dfc->freq_table); > free_dfc: > kfree(dfc); > +free_ops: > + kfree(ops); > > return ERR_PTR(err); > } > @@ -510,11 +518,13 @@ EXPORT_SYMBOL_GPL(devfreq_cooling_em_register); > void devfreq_cooling_unregister(struct thermal_cooling_device *cdev) > { > struct devfreq_cooling_device *dfc; > + const struct thermal_cooling_device_ops *ops; > struct device *dev; > > if (IS_ERR_OR_NULL(cdev)) > return; > > + ops = cdev->ops; > dfc = cdev->devdata; > dev = dfc->devfreq->dev.parent; > > @@ -525,5 +535,6 @@ void devfreq_cooling_unregister(struct thermal_cooling_device *cdev) > > kfree(dfc->freq_table); > kfree(dfc); > + kfree(ops); > } > EXPORT_SYMBOL_GPL(devfreq_cooling_unregister);
Thank you for updating it, LGTM
Reviewed-by: Lukasz Luba <lukasz.luba@arm.com>
Regards, Lukasz
| |