linux-kernel - Re: [PATCH v22 5/7] soc: mediatek: SVS: add debug commands

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, 15 Feb 2022 10:10:57 +0100
From:   AngeloGioacchino Del Regno 
        <angelogioacchino.delregno@...labora.com>
To:     Roger Lu <roger.lu@...iatek.com>,
        Matthias Brugger <matthias.bgg@...il.com>,
        Enric Balletbo Serra <eballetbo@...il.com>,
        Kevin Hilman <khilman@...nel.org>,
        Rob Herring <robh+dt@...nel.org>,
        Nicolas Boichat <drinkcat@...gle.com>,
        Stephen Boyd <sboyd@...nel.org>,
        Philipp Zabel <p.zabel@...gutronix.de>
Cc:     Fan Chen <fan.chen@...iatek.com>,
        HenryC Chen <HenryC.Chen@...iatek.com>,
        Xiaoqing Liu <Xiaoqing.Liu@...iatek.com>,
        Charles Yang <Charles.Yang@...iatek.com>,
        Angus Lin <Angus.Lin@...iatek.com>,
        Mark Rutland <mark.rutland@....com>,
        Nishanth Menon <nm@...com>, devicetree@...r.kernel.org,
        linux-arm-kernel@...ts.infradead.org,
        linux-mediatek@...ts.infradead.org, linux-kernel@...r.kernel.org,
        linux-pm@...r.kernel.org,
        Project_Global_Chrome_Upstream_Group@...iatek.com,
        Guenter Roeck <linux@...ck-us.net>
Subject: Re: [PATCH v22 5/7] soc: mediatek: SVS: add debug commands

Il 15/02/22 10:08, Roger Lu ha scritto:
> Hi AngeloGioacchino,
> 
> Excuse me for the late reply.

Hi Roger,
no worries about that.

> 
> On Mon, 2022-01-31 at 12:11 +0100, AngeloGioacchino Del Regno wrote:
>> Il 27/01/22 04:39, Roger Lu ha scritto:
>>> The purpose of SVS is to help find the suitable voltages
>>> for DVFS. Therefore, if SVS bank voltages are concerned
>>> to be wrong, we can adjust SVS bank voltages by this patch.
>>>
>>> Signed-off-by: Roger Lu <roger.lu@...iatek.com>
>>
>>
>> Hello Roger,
>> I was thinking about what this patch is adding... and I have a few
>> considerations.
>>
>> It's nice to have a debugging mechanism to read the status and dump registers,
>> as
>> that's very helpful when doing heavy debugging of the IP... but adding the
>> possibility to write a voltage offset may be very dangerous: think about the
>> case
>> in which, either for misconfiguration, or for any other reason, the debugfs
>> entry
>> that allows writing voffset becomes user-writable, or a user writes an
>> impossibly
>> high voffset.
>> In case a very low (negative) voffset is entered, the platform would crash
>> (denial
>> of service); if a very high voffset is entered, hardware damage may occur.
>>
>> For this reason, there are two proposals:
>> 1. If you want to keep the debugfs voffset write, please constrain the
>> permissible
>>      voffset to an acceptable range that at least makes it unlikely to damage
>> the HW;
>>      Moreover, since voffset write is a feature that would be used in very
>> limited
>>      debugging cases, I think that this should be implemented over a build-time
>>      configuration barrier... something like CONFIG_MTK_SVS_DEBUG_ALLOW_WRITE,
>> or
>>      similar;
>> 2. Since it's very unlikely for someone to really play that much with a
>> voltage
>>      offset during runtime, and since this looks like something very machine
>> specific
>>      (perhaps addressing board-specific quirks?), I would suggest to add this
>> as a
>>      device-tree parameter instead, such as "mediatek,svs-voffset", as it is
>> indeed
>>      possible to specify both positive or negative values in DT.
>>
>> I would prefer proposal 2, as it looks generally cleaner and way less risky.
> 
> Thanks for raising the considerations and give these great suggestions for us to
> think about. Since these voffset read/write commands are used seldomly, we
> decide to remove them for better system security.
> 

Thank you for this ack, very much appreciated.
Eager to see v23!


>>
>> Regards,
>> Angelo
>