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] [day] [month] [year] [list]

Date:   Tue, 19 Oct 2021 18:26:14 +0200
From:   "Rafael J. Wysocki" <rafael@...nel.org>
To:     Daniel Lezcano <daniel.lezcano@...aro.org>
Cc:     "Rafael J. Wysocki" <rafael@...nel.org>,
        "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 Tue, Oct 19, 2021 at 5:59 PM Daniel Lezcano
<daniel.lezcano@...aro.org> wrote:
>
> 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 ?

It is, except that it may not be straightforward to connect the
message to the specific device in question.