linux-kernel - Re: [PATCH v1] x86/smp: Set up exception handling before cr4

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: <DAF4D431-5596-4FD1-BF8B-D7D753C0810C@zytor.com>
Date: Sun, 8 Feb 2026 11:02:02 -0800
From: Xin Li <xin@...or.com>
To: Dave Hansen <dave.hansen@...el.com>
Cc: linux-kernel@...r.kernel.org, tglx@...utronix.de, mingo@...hat.com,
        bp@...en8.de, dave.hansen@...ux.intel.com, x86@...nel.org,
        hpa@...or.com, peterz@...radead.org, andrew.cooper3@...rix.com,
        sohil.mehta@...el.com, nikunj@....com, thomas.lendacky@....com,
        seanjc@...gle.com, stable@...r.kernel.org
Subject: Re: [PATCH v1] x86/smp: Set up exception handling before cr4_init()


>> + /*
>> +  * Ensure bits set in cr4_pinned_bits are set in CR4.
>> +  *
>> +  * cr4_pinned_bits is a subset of cr4_pinned_mask, which includes
>> +  * the following bits:
>> +  *         X86_CR4_SMEP
>> +  *         X86_CR4_SMAP
>> +  *         X86_CR4_UMIP
>> +  *         X86_CR4_FSGSBASE
>> +  *         X86_CR4_CET
>> +  *         X86_CR4_FRED
>> +  */
>> + cr4_init();
> 
> I'm not as big of a fan of this comment. The next pinned bit that gets
> added will make this stale. Could we try to make this more timeless, please?

Right, it’s still a potential problem.

> 
> I'm also not sure I like the asymmetry of this between the boot and
> secondary CPUs. On a boot CPU, CR4.SMEP will get set via
> identify_boot_cpu() and eventually setup_smep(). On a secondary CPU,
> it'll get set in cr4_init() and *not* in setup_smep().
> 
> This asymmetry is (I think) part of what the root of the problem is here
> and how this bug came to be.

Are you proposing a single CPU initialization function for both BSP and
APs?

It reminds me that tglx proposed the following change when I was fixing
a FRED boot order bug.

diff --git a/arch/x86/kernel/cpu/common.c b/arch/x86/kernel/cpu/common.c
index 6de12b3c1b04..a4735d9b5a1d 100644
--- a/arch/x86/kernel/cpu/common.c
+++ b/arch/x86/kernel/cpu/common.c
@@ -2176,7 +2176,7 @@ static inline void tss_setup_io_bitmap(struct tss_struct *tss)
  * Setup everything needed to handle exceptions from the IDT, including the IST
  * exceptions which use paranoid_entry().
  */
-void cpu_init_exception_handling(void)
+void cpu_init_exception_handling(bool boot_cpu)
 {
 	struct tss_struct *tss = this_cpu_ptr(&cpu_tss_rw);
 	int cpu = raw_smp_processor_id();


So something like

	void cpu_init(bool boot_cpu)

and it gets called on both BSP and APs.

> 
> I really think the current pinning behavior is too invasive. It has zero
> benefit to be pinning CR bits this early in bringup. It's only causing pain.


Maybe someone can explain why it’s added like this before I find time to
dig into it.

I’m curious why cr4_init() is not part of the following cpu_init()? IOW,
why does it need to be called so early in the existing code?


> 
> I _really_ think we need a defined per-cpu point where pinning comes
> into effect. Marking the CPU online is one idea.
> 
> Thoughts?

It seems a good fit.  Just that {on,off}line() are not called on BSP (not
a real problem).

Question is that who would work on it ;) ?