linux-kernel - Re: [PATCH] cpufreq: powernv: Check negative value returned by cpufreq_table_find_index

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: Windows password security audit tool. GUI, reports in PDF.

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

Message-ID: <87vaeqqye8.fsf@concordia.ellerman.id.au>
Date:   Thu, 22 Feb 2018 00:13:03 +1100
From:   Michael Ellerman <mpe@...erman.id.au>
To:     "Rafael J. Wysocki" <rafael@...nel.org>,
        Viresh Kumar <viresh.kumar@...aro.org>
Cc:     Shilpasri G Bhat <shilpa.bhat@...ux.vnet.ibm.com>,
        "Rafael J. Wysocki" <rjw@...ysocki.net>,
        Linux PM <linux-pm@...r.kernel.org>,
        Linux Kernel Mailing List <linux-kernel@...r.kernel.org>,
        linuxppc-dev <linuxppc-dev@...ts.ozlabs.org>
Subject: Re: [PATCH] cpufreq: powernv: Check negative value returned by cpufreq_table_find_index_dl()

"Rafael J. Wysocki" <rafael@...nel.org> writes:

> On Wed, Feb 21, 2018 at 6:54 AM, Viresh Kumar <viresh.kumar@...aro.org> wrote:
>> On 21-02-18, 16:39, Michael Ellerman wrote:
>>> Viresh Kumar <viresh.kumar@...aro.org> writes:
>>
>>> > AFAICT, you will get -1 here only if the freq table had no valid
>>> > frequencies (or the freq table is empty). Why would that happen ?
>>>
>>> Bugs?
>>
>> The cupfreq driver shouldn't have registered itself in that case (i.e.
>> if the cpufreq table is empty).
>
> To be precise, ->init() should fail as that's where the table is
> created.  The registration fails as a result then.
>
> But what if the bug is that ->init() doesn't fail when it should?
>
> I guess the core could double check the frequency table after ->init()
> if ->target_index is not NULL.
>
> The overall point here is that if you get a negative index in
> ->fast_switch(), that's way too late anyway and we should be able to
> catch that error much earlier.

OK.

Still it's one thing for the driver to print a warning and bail out,
it's another to access off the front of an array and keep running using
some junk values, or oops (though not in this case because the array
happens to be static).

cheers