| 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
| ||
|
Message-ID: <d8ff7f3b-b5d9-acb4-1e10-33589f8c14cb@arm.com>
Date: Tue, 29 Mar 2022 07:39:14 +0100
From: Lukasz Luba <lukasz.luba@....com>
To: Kant Fan <kant@...winnertech.com>
Cc: Amit Kucheria <amitk@...nel.org>, Zhang Rui <rui.zhang@...el.com>,
"open list:THERMAL" <linux-pm@...r.kernel.org>,
open list <linux-kernel@...r.kernel.org>,
"supporter:THERMAL" <rafael@...nel.org>,
"supporter:THERMAL" <daniel.lezcano@...aro.org>,
allwinner-opensource-support
<allwinner-opensource-support@...winnertech.com>
Subject: Re: [PATCH] thermal: devfreq_cooling: use local ops instead of global
ops
On 3/25/22 10:43, Kant Fan wrote:
> On 14/03/2022 21:41, Lukasz Luba wrote:
>> Hi Kant,
>>
>> On 3/12/22 04:59, 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.
>>>
>>> Signed-off-by: Kant Fan <kant@...winnertech.com>
>>
>> Thank you for finding this issue. This was also an issue since the
>> beginning of that code. The modified global ops after first registration
>> which went through, was also previously there. Thus, we would need two
>> different patches for stable kernels.
>>
>> For this one, please add the tag:
>> Fixes: 615510fe13bd2 ("thermal: devfreq_cooling: remove old power
>> model and use EM")
>>
>> This patch would also go via stable tree for kernels v5.11+
>> Please read the process how to send a patch which will be merged to the
>> stable tree.
>>
>> There will be a need to create another patch(es) for stable kernels with
>> Fixes: a76caf55e5b35 ("thermal: Add devfreq cooling")
>> In those kernels also the global ops is modified and might not support
>> properly many cooling devices. It's present in other stable kernels:
>> v5.10 and older
>>
>>> ---
>>> 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);
>>
>> The fix looks good.
>
> Hi Lukasz,
> Thanks for your advice. According to that, I made two separate patches
> for mainline and the stable trees:
> The first patch (patchwork.kernel.org: Message ID:
> 20220325073030.91919-1-kant@...winnertech.com) is for mainline. I added
> the 'fix' tag and 'Cc: stable@...r.kernel.org # 5.13+' to remind which
> stable trees should be back-ported.
> The second patch (patchwork.kernel.org: Message ID:
> 20220325094436.101419-1-kant@...winnertech.com) is for stable tree v5.10
> and older. I added an upstream commit ID to indicate where the patch
> comes from. I also added 'Cc: stable@...r.kernel.org # 4.4+' to remind
> which stable trees should be back-ported.
> Please check if they are correct. Thank you.
Thank you for sending them. Let me have a look.
Powered by blists - more mailing lists