linux-kernel - Re: [PATCH v3] PM: EM: Fix potential division-by-zero error in em_compute

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: <df494648-b0c1-402b-8644-b50f20011c5f@arm.com>
Date: Wed, 16 Apr 2025 16:26:03 +0100
From: Lukasz Luba <lukasz.luba@....com>
To: "Rafael J. Wysocki" <rafael@...nel.org>
Cc: linux-pm@...r.kernel.org, linux-kernel@...r.kernel.org,
 Yaxiong Tian <tianyaxiong@...inos.cn>, Yaxiong Tian <iambestgod@...com>
Subject: Re: [PATCH v3] PM: EM: Fix potential division-by-zero error in
 em_compute_costs()



On 4/16/25 12:58, Rafael J. Wysocki wrote:
> On Wed, Apr 16, 2025 at 4:57 AM Yaxiong Tian <iambestgod@...com> wrote:
>>
>> 在 2025/4/16 01:17, Rafael J. Wysocki 写道:
>>> On Mon, Apr 14, 2025 at 11:09 AM Yaxiong Tian <iambestgod@...com> wrote:
>>>>
>>>> From: Yaxiong Tian <tianyaxiong@...inos.cn>
>>>>
>>>> When the device is of a non-CPU type, table[i].performance won't be
>>>> initialized in the previous em_init_performance(), resulting in division
>>>> by zero when calculating costs in em_compute_costs().
>>>>
>>>> Since the 'cost' algorithm is only used for EAS energy efficiency
>>>> calculations and is currently not utilized by other device drivers, we
>>>> should add the _is_cpu_device(dev) check to prevent this division-by-zero
>>>> issue.
>>>>
>>>> Fixes: <1b600da51073> ("PM: EM: Optimize em_cpu_energy() and remove division")
>>>
>>> Please look at the Fixes: tags in the kernel git history.  They don't
>>> look like the one above.
>>>
>> Yes, there's an extra '<>' here.
>>
>>>> Signed-off-by: Yaxiong Tian <tianyaxiong@...inos.cn>
>>>> ---
>>>>    kernel/power/energy_model.c | 4 ++--
>>>>    1 file changed, 2 insertions(+), 2 deletions(-)
>>>>
>>>> diff --git a/kernel/power/energy_model.c b/kernel/power/energy_model.c
>>>> index d9b7e2b38c7a..fc972cc1fc12 100644
>>>> --- a/kernel/power/energy_model.c
>>>> +++ b/kernel/power/energy_model.c
>>>> @@ -235,7 +235,7 @@ static int em_compute_costs(struct device *dev, struct em_perf_state *table,
>>>>
>>>>           /* Compute the cost of each performance state. */
>>>>           for (i = nr_states - 1; i >= 0; i--) {
>>>> -               unsigned long power_res, cost;
>>>> +               unsigned long power_res, cost = 0;
>>>>
>>>>                   if ((flags & EM_PERF_DOMAIN_ARTIFICIAL) && cb->get_cost) {
>>>>                           ret = cb->get_cost(dev, table[i].frequency, &cost);
>>>> @@ -244,7 +244,7 @@ static int em_compute_costs(struct device *dev, struct em_perf_state *table,
>>>>                                           cost, ret);
>>>>                                   return -EINVAL;
>>>>                           }
>>>> -               } else {
>>>> +               } else if (_is_cpu_device(dev)) {
>>>
>>> Can't you just check this upfront at the beginning of the function and
>>> make it bail out if dev is not a CPU device?
>>>
>> Sure, But the current implementation applies em_compute_costs() to both
>> non-CPU devices and CPU devices.
> 
> Maybe it shouldn't do that for non-CPU ones?

It shouldn't call this cost computation for non-CPU devices.
Let me check that.