| lists.openwall.net | lists / announce owl-users owl-dev john-users john-dev passwdqc-users yescrypt popa3d-users / oss-security kernel-hardening musl sabotage tlsify passwords / crypt-dev xvendor / Bugtraq Full-Disclosure linux-kernel linux-netdev linux-ext4 linux-hardening linux-cve-announce PHC | |
|
Open Source and information security mailing list archives
| ||
|
Date: Wed, 13 Apr 2022 16:41:26 +0100
From: Lukasz Luba <lukasz.luba@....com>
To: "Rafael J. Wysocki" <rafael@...nel.org>
Cc: Amit Kucheria <amitk@...nel.org>,
"Zhang, Rui" <rui.zhang@...el.com>,
Linux PM <linux-pm@...r.kernel.org>,
Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
allwinner-opensource-support@...winnertech.com,
Stable <stable@...r.kernel.org>,
Daniel Lezcano <daniel.lezcano@...aro.org>,
Ionela Voinescu <ionela.voinescu@....com>,
Kant Fan <kant@...winnertech.com>
Subject: Re: [PATCH v2] thermal: devfreq_cooling: use local ops instead of
global ops
On 4/13/22 16:06, Lukasz Luba wrote:
>
>
> On 4/13/22 15:58, Rafael J. Wysocki wrote:
>> On Fri, Mar 25, 2022 at 10:02 AM Lukasz Luba <lukasz.luba@....com> wrote:
>>>
>>> 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@...r.kernel.org # 5.13+
>>>> Signed-off-by: Kant Fan <kant@...winnertech.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@....com>
>>
>> Applied as 5.19 material.
>>
>> Lukasz, this had a conflict with your EM series, please double check
>> if my resolution in the bleeding-edge branch is correct.
>
> OK, I'll let you know after I fetch and build that branch.
I've read the code and confirm you've do this correctly.
I've also built that branch with ENERGY_MODEL and DEVFREQ_COOLING
configs set - no issues observed.
Later this week I would use it for some other development
so I will test it as well.
Thank you for solving this!
Regards,
Lukasz
Powered by blists - more mailing lists