linux-kernel - Re: [tip:x86/alternatives] [x86/alternatives] ee8962082a: WARNING:at_arch/x86/kernel/cpu/cpuid-deps.c:#do_clear_cpu

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: <ZjE7DkTBSbPlBN8k@google.com>
Date: Tue, 30 Apr 2024 11:40:14 -0700
From: Sean Christopherson <seanjc@...gle.com>
To: Borislav Petkov <bp@...en8.de>
Cc: kernel test robot <oliver.sang@...el.com>, oe-lkp@...ts.linux.dev, lkp@...el.com, 
	linux-kernel@...r.kernel.org, x86@...nel.org, Ingo Molnar <mingo@...nel.org>, 
	Srikanth Aithal <sraithal@....com>
Subject: Re: [tip:x86/alternatives] [x86/alternatives] ee8962082a: WARNING:at_arch/x86/kernel/cpu/cpuid-deps.c:#do_clear_cpu_cap

On Tue, Apr 30, 2024, Borislav Petkov wrote:
> + Sean.
> > [ 0.055225][ T0] ? __warn (kernel/panic.c:694) 
> > [ 0.055225][ T0] ? do_clear_cpu_cap (arch/x86/kernel/cpu/cpuid-deps.c:118 (discriminator 1)) 
> > [ 0.055225][ T0] ? report_bug (lib/bug.c:180 lib/bug.c:219) 
> > [ 0.055225][ T0] ? handle_bug (arch/x86/kernel/traps.c:239 (discriminator 1)) 
> > [ 0.055225][ T0] ? exc_invalid_op (arch/x86/kernel/traps.c:260 (discriminator 1)) 
> > [ 0.055225][ T0] ? asm_exc_invalid_op (arch/x86/include/asm/idtentry.h:621) 
> > [ 0.055225][ T0] ? do_clear_cpu_cap (arch/x86/kernel/cpu/cpuid-deps.c:118 (discriminator 1)) 
> > [ 0.055225][ T0] ? __pfx_do_clear_cpu_cap (arch/x86/kernel/cpu/cpuid-deps.c:109) 
> > [ 0.055225][ T0] init_ia32_feat_ctl (arch/x86/kernel/cpu/feat_ctl.c:181)
> 
> Yap, works as designed:

..

> won't work as expected because the patching has already happened.
> 
> And I'm not sure even the dynamic testing *cpu_has() does will always
> work as we do this dance in get_cpu_cap() with forced flags.
> 
> So, I'm thinking init_ia32_feat_ctl() should run in early_init_intel()
> which is before alternatives.
> 
> And looking at init_ia32_feat_ctl(), all it does is set and clear
> a bunch of bits so I think it should be ok.

Hmm, I don't think the problem is that init_ia32_feat_ctl() is called too late.
It too is called from the BSP prior to alternative_instructions():

  arch_cpu_finalize_init()
  |
  -> identify_boot_cpu()
     |
     -> identify_cpu()
        |
        -> .c_init() => init_intel()

Ah, and the WARN even specifically checks for the case where there's divergence
from the boot CPU:

	if (boot_cpu_has(feature))
		WARN_ON(alternatives_patched);

So I think this is a "real" warning about a misconfigured system, where VMX is
fully configured in MSR_IA32_FEAT_CTL on the boot CPU, but is disabled on a
secondary CPU.

FWIW, KVM will play nice with such a setup, as KVM specifically checks that VMX
is fully enabled all CPUs, but I would still consider this a misconfigured system.

> > [ 0.055225][ T0] init_intel (arch/x86/include/asm/msr.h:146 arch/x86/include/asm/msr.h:300 arch/x86/kernel/cpu/intel.c:583 arch/x86/kernel/cpu/intel.c:687) 
> > [ 0.055225][ T0] identify_cpu (arch/x86/kernel/cpu/common.c:1824) 
> > [ 0.055225][ T0] identify_secondary_cpu (arch/x86/kernel/cpu/common.c:1949) 
> > [ 0.055225][ T0] smp_store_cpu_info (arch/x86/kernel/smpboot.c:333) 
> > [ 0.055225][ T0] start_secondary (arch/x86/kernel/smpboot.c:197 arch/x86/kernel/smpboot.c:281) 
> > [ 0.055225][ T0] ? __pfx_start_secondary (arch/x86/kernel/smpboot.c:231) 
> > [ 0.055225][ T0] common_startup_64 (arch/x86/kernel/head_64.S:421) 
> > [    0.055225][    T0]  </TASK>
> > [    0.055225][    T0] ---[ end trace 0000000000000000 ]---