linux-kernel - Re: [PATCH 1/2] cpupower: Fix amd cpu (family

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

Message-ID: <87e43451-e7d3-877e-7081-92838ae699e6@linuxfoundation.org>
Date:   Fri, 23 Apr 2021 16:26:24 -0600
From:   Shuah Khan <skhan@...uxfoundation.org>
To:     徐福海 <xufuhai1992@...il.com>, shuah@...nel.org,
        Thomas Renninger <trenn@...e.com>
Cc:     linux-pm@...r.kernel.org, linux-kernel@...r.kernel.org,
        lishujin@...ishou.com, xufuhai <xufuhai@...ishou.com>,
        Shuah Khan <skhan@...uxfoundation.org>
Subject: Re: [PATCH 1/2] cpupower: Fix amd cpu (family < 0x17) active state
 issue

On 4/19/21 8:27 PM, 徐福海 wrote:
> From: xufuhai<xufuhai@...ishou.com>
> 
> For the old  AMD processor (family < 0x17), cpupower will call the
> amd_pci_get_num_boost_states function, but for the non-root user
> pci_read_byte function (implementation comes from the psutil library),
> val will be set to 0xff, indicating that there is no read function
> callback. At this time, the original logic will set the cpupower turbo
> active state to yes. This is an obvious issue~
> 
> Reproduce procedure:
> 	cpupower frequency-info
> 
> Reported-by: yangrui<yangrui@...ishou.com>
> Signed-off-by: xufuhai<xufuhai@...ishou.com>

Also your Signed-off-by should match the from address.
There is a mismatch between the two.

> Signed-off-by: chenguanqiao<chenguanqiao@...ishou.com>
> Signed-off-by: lishujin<lishujin@...ishou.com>
> Reviewed-by: Thomas Renninger<trenn@...e.com>
> ---
>   tools/power/cpupower/utils/helpers/amd.c | 7 +++++++
>   1 file changed, 7 insertions(+)
> 
> diff --git a/tools/power/cpupower/utils/helpers/amd.c b/tools/power/cpupower/utils/helpers/amd.c
> index 97f2c857048e..6f9504906afa 100644
> --- a/tools/power/cpupower/utils/helpers/amd.c
> +++ b/tools/power/cpupower/utils/helpers/amd.c
> @@ -137,6 +137,13 @@ int amd_pci_get_num_boost_states(int *active, int *states)
>   		return -ENODEV;
>   
>   	val = pci_read_byte(device, 0x15c);
> +
> +	/* If val is 0xff, meaning has no permisson to
> +	 * get the boost states, return -1
> +	 */
> +	if (val == 0xff)
> +		return -1;
> +
>   	if (val & 3)
>   		*active = 1;
>   	else
> -- 
> 2.24.3 (Apple Git-128)
> 

I am seeing two patches with the same commit summary,
should these two be a singles patch?

https://patchwork.kernel.org/project/linux-pm/patch/6e35df20-753a-6c9c-8786-3fc87cdd17ba@gmail.com/

Please combine the two and send single patch if they fix the
same problem. If not, please change the commit log to reflect
the difference.

thanks,
-- Shuah