linux-kernel - Re: [PATCH v2 2/2] thermal/core: Make deprecated cooling device state change from userspace

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

Hash Suite for Android: free password hash cracker in your pocket

[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]

Date:   Tue, 19 Oct 2021 17:59:23 +0200
From:   Daniel Lezcano <daniel.lezcano@...aro.org>
To:     "Rafael J. Wysocki" <rafael@...nel.org>
Cc:     "Rafael J. Wysocki" <rjw@...ysocki.net>,
        Lukasz Luba <lukasz.luba@....com>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        Linux PM <linux-pm@...r.kernel.org>,
        Zhang Rui <rui.zhang@...el.com>,
        Amit Kucheria <amitk@...nel.org>
Subject: Re: [PATCH v2 2/2] thermal/core: Make deprecated cooling device state
 change from userspace

On 19/10/2021 17:43, Rafael J. Wysocki wrote:
> On Tue, Oct 19, 2021 at 5:40 PM Rafael J. Wysocki <rafael@...nel.org>
> wrote:
>> 
>> On Tue, Oct 19, 2021 at 5:02 PM Daniel Lezcano 
>> <daniel.lezcano@...aro.org> wrote:
>>> 
>>> The cooling devices have their cooling device set_cur_state 
>>> read-writable all the time in the sysfs directory, thus allowing
>>> the userspace to act on it.
>>> 
>>> The thermal framework is wrongly used by userspace as a power
>>> capping framework by acting on the cooling device opaque state.
>>> This one then competes with the in-kernel governor decision.
>>> 
>>> We have seen in out-of-tree kernels, a big number of devices
>>> which are abusely declaring themselves as cooling device just to
>>> act on their power.
>>> 
>>> The role of the thermal framework is to protect the junction 
>>> temperature of the silicon. Letting the userspace to play with a 
>>> cooling device is invalid and potentially dangerous.
>>> 
>>> The powercap framework is the right framework to do power capping
>>> and moreover it deals with the aggregation via the dev pm qos.
>>> 
>>> As the userspace governor is marked deprecated and about to be 
>>> removed, there is no point to keep this file writable also in
>>> the future.
>>> 
>>> Emit a warning and deprecate the interface.
>>> 
>>> Signed-off-by: Daniel Lezcano <daniel.lezcano@...aro.org> --- 
>>> drivers/thermal/thermal_sysfs.c | 2 ++ 1 file changed, 2
>>> insertions(+)
>>> 
>>> diff --git a/drivers/thermal/thermal_sysfs.c
>>> b/drivers/thermal/thermal_sysfs.c index
>>> 1c4aac8464a7..730f1361dbef 100644 ---
>>> a/drivers/thermal/thermal_sysfs.c +++
>>> b/drivers/thermal/thermal_sysfs.c @@ -610,6 +610,8 @@
>>> cur_state_store(struct device *dev, struct device_attribute
>>> *attr, unsigned long state; int result;
>>> 
>>> +       pr_warn_once("Setting cooling device state is
>>> deprecated\n");
>> 
>> Maybe dev_warn_once() even?  I guess it won\t hurt to print it
>> once per the affected device?

Actually, there is no difference except it will be prefixed by the
device name.

As the static variable to write once the message is local to the
function, not the dev, the warning will appear only once even if we use
dev, or cdev->device.

The other calls in the file are pr_warn. I suggest to stay consistent in
this case. Is that fine ?

>>> + if (sscanf(buf, "%ld\n", &state) != 1) return -EINVAL;
>>> 
>>> --
> 
> Also I think that changing the subject to something like "Deprecate 
> changing cooling device state from userspace" would help.

Ok


-- 
<http://www.linaro.org/> Linaro.org │ Open source software for ARM SoCs

Follow Linaro:  <http://www.facebook.com/pages/Linaro> Facebook |
<http://twitter.com/#!/linaroorg> Twitter |
<http://www.linaro.org/linaro-blog/> Blog