linux-kernel - Re: [PATCH v1 RESEND] EDAC/bluefield

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: <20241016165027.GTZw_u0wpcd48VwbMA@fat_crate.local>
Date: Wed, 16 Oct 2024 18:50:27 +0200
From: Borislav Petkov <bp@...en8.de>
To: David Thompson <davthompson@...dia.com>
Cc: tony.luck@...el.com, james.morse@....com, mchehab@...nel.org,
	rric@...nel.org, linux-edac@...r.kernel.org, shravankr@...dia.com,
	linux-kernel@...r.kernel.org
Subject: Re: [PATCH v1 RESEND] EDAC/bluefield - fix potential integer overflow

On Mon, Sep 30, 2024 at 11:10:56AM -0400, David Thompson wrote:
> The 64-bit argument for the "get DIMM info" SMC call consists of
> "mem_ctrl_idx" left-shifted 16 bits and OR-ed with DIMM index.
> With "mem_ctrl_idx" defined as 32-bits wide the left-shift operation
> truncates the upper 16 bits of information during the calculation
> of the SMC argument. The "mem_ctrl_idx" stack variable must be
> defined as 64-bits wide to prevent any potential integer overflow,
> i.e. loss of data from upper 16 bits.
> 
> Fixes: 82413e562ea6 ("EDAC, mellanox: Add ECC support for BlueField DDR4")
> Reviewed-by: Shravan Kumar Ramani <shravankr@...dia.com>
> Signed-off-by: David Thompson <davthompson@...dia.com>
> ---
>  drivers/edac/bluefield_edac.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/edac/bluefield_edac.c b/drivers/edac/bluefield_edac.c
> index 5b3164560648..0e539c107351 100644
> --- a/drivers/edac/bluefield_edac.c
> +++ b/drivers/edac/bluefield_edac.c
> @@ -180,7 +180,7 @@ static void bluefield_edac_check(struct mem_ctl_info *mci)
>  static void bluefield_edac_init_dimms(struct mem_ctl_info *mci)
>  {
>  	struct bluefield_edac_priv *priv = mci->pvt_info;
> -	int mem_ctrl_idx = mci->mc_idx;
> +	u64 mem_ctrl_idx = mci->mc_idx;
>  	struct dimm_info *dimm;
>  	u64 smc_info, smc_arg;
>  	int is_empty = 1, i;
> -- 

Is this something you're hitting in real workloads so that it needs to go to
stable or is it rather something caught through code review or so?

Thx.

-- 
Regards/Gruss,
    Boris.

https://people.kernel.org/tglx/notes-about-netiquette