linux-kernel - Re: [PATCH v3 1/6] regulator: core: validate selector against linear_min

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]

Message-ID: <f06c90ca-11c8-961d-1461-a9486933a1a3@microchip.com>
Date:   Tue, 24 Nov 2020 11:14:54 +0000
From:   <Claudiu.Beznea@...rochip.com>
To:     <jonathanh@...dia.com>, <lgirdwood@...il.com>, <broonie@...nel.org>
CC:     <s.hauer@...gutronix.de>, <ttynkkynen@...dia.com>,
        <linus.walleij@...aro.org>, <axel.lin@...ics.com>,
        <linux-kernel@...r.kernel.org>,
        <linux-arm-kernel@...ts.infradead.org>,
        <linux-tegra@...r.kernel.org>
Subject: Re: [PATCH v3 1/6] regulator: core: validate selector against
 linear_min_sel

Hi Jon,

On 24.11.2020 11:36, Jon Hunter wrote:
> EXTERNAL EMAIL: Do not click links or open attachments unless you know the content is safe
> 
> On 13/11/2020 15:21, Claudiu Beznea wrote:
>> There are regulators who's min selector is not zero. Selectors loops
>> (looping b/w zero and regulator::desc::n_voltages) might throw errors
>> because invalid selectors are used (lower than
>> regulator::desc::linear_min_sel). For this situations validate selectors
>> against regulator::desc::linear_min_sel.
> 
> 
> After this commit was merged, I noticed a regression in the DFLL (CPU
> clock source) on Tegra124. The DFLL driver
> (drivers/clk/tegra/clk-dfll.c) calls regulator_list_voltage() in a loop
> to determine the selector for a given voltage (see function
> find_vdd_map_entry_exact()).
> 
> Currently, the DFLL driver queries the number of voltages provided by
> the regulator by calling regulator_count_voltages() and then starting
> from 0, iterates through the number of voltages to find the selector
> value for the voltage it is looking for by calling
> regulator_list_voltage(). It assumes that any negative value returned by
> calling regulator_list_voltage() is an error and this will cause the
> loop up to terminate.
> 
> In this case the regulator in question is the as3722 and the
> linear_min_sel for this regulator is 1 and so when the DFLL driver calls
> regulator_list_voltage() with a selector value of 0 it now returns a
> negative error code, as expected by this change, and this terminates the
> loop up in the DFLL driver. So I can clearly see why this is happening
> and I could fix up the DFLL driver to avoid this.
> 
> Before doing so, I wanted to ask if that is the correct fix here,
> because it seems a bit odd that regulator_count_voltages() returns N
> voltages, but if the min selector value is greater than 0, then actually
> there are less than N. However, changing the number of voltages
> supported by the regulator to be N - linear_min_sel does not make sense
> either because then we need to know the linear_min_sel in order to
> determine the first valid voltage.
> 
> In case of the as3722, the value 0 means that the regulator is powered
> down. So it is a valid setting and equates to 0 volts at the output AFAICT.
> 
> Please let me know your thoughts are the correct way to fix this up.

I would say that a solution would be to have a new helper to retrieve the
linear_min_sel (e.g. regulator_min_sel()) and use this for all the
consumers of regulator_list_voltage() and the other APIs that patch
"regulator: core: validate selector against linear_min_sel" has changed
(regulator_list_voltage_table(), regulator_set_voltage_time()). With this
change the loop in find_vdd_map_entry_exact() should be b/w
regulator_min_sel() and regulator_count_voltages().

Maybe Mark has a better solution for this.

Thank you,
Claudiu

> 
> Thanks
> Jon
> 
> --
> nvpublic
>