linux-kernel - Re: [PATCH] platform/x86: ISST: Correct command storage data length

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

Message-ID: <4049a875-fb64-c84d-c092-daa11fcc9a6c@linux.intel.com>
Date: Tue, 1 Apr 2025 14:35:01 +0300 (EEST)
From: Ilpo Järvinen <ilpo.jarvinen@...ux.intel.com>
To: Srinivas Pandruvada <srinivas.pandruvada@...ux.intel.com>
cc: Hans de Goede <hdegoede@...hat.com>, platform-driver-x86@...r.kernel.org, 
    LKML <linux-kernel@...r.kernel.org>, stable@...r.kernel.org
Subject: Re: [PATCH] platform/x86: ISST: Correct command storage data
 length

On Fri, 28 Mar 2025, Srinivas Pandruvada wrote:

> After resume/online turbo limit ratio (TRL) is restored partially if
> the admin explicitly changed TRL from user space.
> 
> A hash table is used to store SST mail box and MSR settings when modified
> to restore those settings after resume or online. This uses a struct
> isst_cmd field "data" to store these settings. This is a 64 bit field.
> But isst_store_new_cmd() is only assigning as u32. This results in
> truncation of 32 bits.
> 
> Change the argument to u64 from u32.
> 
> Fixes: f607874f35cb ("platform/x86: ISST: Restore state on resume")
> Signed-off-by: Srinivas Pandruvada <srinivas.pandruvada@...ux.intel.com>
> Cc: stable@...r.kernel.org

Thanks, I've applied this to the review-ilpo-fixes branch.

While reviewing this, I noticed full_cmd is currently typed as int but I 
think it should be u32 to match the type in the struct.

The construction of full_cmd also open codes FIELD_PREP() and doesn't use 
named defines for the fields. And ->cmd is also decomposed in 
isst_mbox_resume_command() which should use FIELD_GET() and the same 
defines.

> ---
>  drivers/platform/x86/intel/speed_select_if/isst_if_common.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/platform/x86/intel/speed_select_if/isst_if_common.c b/drivers/platform/x86/intel/speed_select_if/isst_if_common.c
> index dbcd3087aaa4..31239a93dd71 100644
> --- a/drivers/platform/x86/intel/speed_select_if/isst_if_common.c
> +++ b/drivers/platform/x86/intel/speed_select_if/isst_if_common.c
> @@ -84,7 +84,7 @@ static DECLARE_HASHTABLE(isst_hash, 8);
>  static DEFINE_MUTEX(isst_hash_lock);
>  
>  static int isst_store_new_cmd(int cmd, u32 cpu, int mbox_cmd_type, u32 param,
> -			      u32 data)
> +			      u64 data)
>  {
>  	struct isst_cmd *sst_cmd;

-- 
 i.