| 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
| ||
|
Message-ID: <20240729153008.GA685493@thelio-3990X>
Date: Mon, 29 Jul 2024 08:30:08 -0700
From: Nathan Chancellor <nathan@...nel.org>
To: Borislav Petkov <bp@...en8.de>
Cc: kernel test robot <lkp@...el.com>, 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
Hi Boris,
On Mon, Jul 29, 2024 at 01:26:14PM +0200, Borislav Petkov wrote:
> + 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.
Right, as clang does not perform interprocedural analysis for the sake
of warnings. That's partly why GCC's version of this warning was
disabled in commit 78a5255ffb6a ("Stop the ad-hoc games with
-Wno-maybe-initialized").
> And that case is handled properly in this function.
While it may be properly handled now, I think this pattern of
conditionally avoiding using a variable uninitialized is dubious.
> So, unless I'm missing something else, it's a good thing this warning is
> behind a W=1. Keep it there.
It's not behind W=1, this happens in a normal build:
$ make -skj"$(nproc)" ARCH=x86_64 LLVM=1 mrproper defconfig arch/x86/kernel/cpu/microcode/amd.o
arch/x86/kernel/cpu/microcode/amd.c:714:6: error: variable 'equiv_id' is used uninitialized whenever 'if' condition is false [-Werror,-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 error generated.
Even GCC would warn about this code if not for the commit I mentioned
above:
$ make -skj"$(nproc)" ARCH=x86_64 CROSS_COMPILE=x86_64-linux- KCFLAGS=-Wmaybe-uninitialized mrproper defconfig arch/x86/kernel/cpu/microcode/amd.o
In function 'cache_find_patch',
inlined from 'find_patch' at arch/x86/kernel/cpu/microcode/amd.c:720:9:
arch/x86/kernel/cpu/microcode/amd.c:647:20: error: 'equiv_id' may be used uninitialized [-Werror=maybe-uninitialized]
647 | if (patch_cpus_equivalent(p, &n))
| ^
arch/x86/kernel/cpu/microcode/amd.c: In function 'find_patch':
arch/x86/kernel/cpu/microcode/amd.c:706:13: note: 'equiv_id' was declared here
706 | u16 equiv_id;
| ^~~~~~~~
cc1: all warnings being treated as errors
So I guess you can just ignore this if you want but others (maybe even
Linus) will likely notice this and report it as well.
Cheers,
Nathan
Powered by blists - more mailing lists