linux-kernel - Re: [tip:x86/microcode 1/1] arch/x86/kernel/cpu/microcode/amd.c:714:6: warning: variable 'equiv_id' is used uninitialized whenever 'if' condition is false

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: <20240729112614.GBZqd8Vu27mFVSHynA@fat_crate.local>
Date: Mon, 29 Jul 2024 13:26:14 +0200
From: Borislav Petkov <bp@...en8.de>
To: kernel test robot <lkp@...el.com>,
	Nathan Chancellor <nathan@...nel.org>
Cc: llvm@...ts.linux.dev, oe-kbuild-all@...ts.linux.dev,
	linux-kernel@...r.kernel.org, x86@...nel.org
Subject: Re: [tip:x86/microcode 1/1]
 arch/x86/kernel/cpu/microcode/amd.c:714:6: warning: variable 'equiv_id' is
 used uninitialized whenever 'if' condition is false

+ Nathan.

On Mon, Jul 29, 2024 at 07:04:51PM +0800, kernel test robot wrote:
> tree:   https://git.kernel.org/pub/scm/linux/kernel/git/tip/tip.git x86/microcode
> head:   94838d230a6c835ced1bad06b8759e0a5f19c1d3
> commit: 94838d230a6c835ced1bad06b8759e0a5f19c1d3 [1/1] x86/microcode/AMD: Use the family,model,stepping encoded in the patch ID
> config: i386-buildonly-randconfig-001-20240729 (https://download.01.org/0day-ci/archive/20240729/202407291815.gJBST0P3-lkp@intel.com/config)
> compiler: clang version 18.1.5 (https://github.com/llvm/llvm-project 617a15a9eac96088ae5e9134248d8236e34b91b1)
> reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240729/202407291815.gJBST0P3-lkp@intel.com/reproduce)
> 
> If you fix the issue in a separate patch/commit (i.e. not just a new version of
> the same patch/commit), kindly add following tags
> | Reported-by: kernel test robot <lkp@...el.com>
> | Closes: https://lore.kernel.org/oe-kbuild-all/202407291815.gJBST0P3-lkp@intel.com/
> 
> All warnings (new ones prefixed by >>):
> 
> >> arch/x86/kernel/cpu/microcode/amd.c:714:6: warning: variable 'equiv_id' is used uninitialized whenever 'if' condition is false [-Wsometimes-uninitialized]
>      714 |         if (x86_family(bsp_cpuid_1_eax) < 0x17) {
>          |             ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>    arch/x86/kernel/cpu/microcode/amd.c:720:31: note: uninitialized use occurs here
>      720 |         return cache_find_patch(uci, equiv_id);
>          |                                      ^~~~~~~~
>    arch/x86/kernel/cpu/microcode/amd.c:714:2: note: remove the 'if' if its condition is always true
>      714 |         if (x86_family(bsp_cpuid_1_eax) < 0x17) {
>          |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
>    arch/x86/kernel/cpu/microcode/amd.c:706:14: note: initialize the variable 'equiv_id' to silence this warning
>      706 |         u16 equiv_id;
>          |                     ^
>          |                      = 0
>    1 warning generated.
> 
> 
> vim +714 arch/x86/kernel/cpu/microcode/amd.c
> 
>    701	
>    702	static struct ucode_patch *find_patch(unsigned int cpu)
>    703	{
>    704		struct ucode_cpu_info *uci = ucode_cpu_info + cpu;
>    705		u32 rev, dummy __always_unused;
>    706		u16 equiv_id;
>    707	
>    708		/* fetch rev if not populated yet: */
>    709		if (!uci->cpu_sig.rev) {
>    710			rdmsr(MSR_AMD64_PATCH_LEVEL, rev, dummy);
>    711			uci->cpu_sig.rev = rev;
>    712		}
>    713	
>  > 714		if (x86_family(bsp_cpuid_1_eax) < 0x17) {
>    715			equiv_id = find_equiv_id(&equiv_table, uci->cpu_sig.sig);
>    716			if (!equiv_id)
>    717				return NULL;
>    718		}
>    719	
>    720		return cache_find_patch(uci, equiv_id);
>    721	}
>    722	

That's a false positive, I think.

clang can't see that equiv_id is used in cache_find_patch() ->
patch_cpus_equivalent() *only* in the

	if (x86_family(bsp_cpuid_1_eax) < 0x17)

case.

And that case is handled properly in this function.

So, unless I'm missing something else, it's a good thing this warning is
behind a W=1. Keep it there.

Thx.

-- 
Regards/Gruss,
    Boris.

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