| 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: <a8c5008a-31f7-953d-22c8-203a2a14d432@allwinnertech.com>
Date: Fri, 25 Mar 2022 18:43:31 +0800
From: Kant Fan <kant@...winnertech.com>
To: Lukasz Luba <lukasz.luba@....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 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.
Kant Fan
Powered by blists - more mailing lists