linux-kernel - Re: [PATCHv9 04/16] x86/cpu: Defer CR pinning setup until core initcall

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] [day] [month] [year] [list]

Message-ID: <202508021149.B4BFF8D1@keescook>
Date: Sat, 2 Aug 2025 11:51:28 -0700
From: Kees Cook <kees@...nel.org>
To: Dave Hansen <dave.hansen@...el.com>
Cc: Sohil Mehta <sohil.mehta@...el.com>,
	Thomas Gleixner <tglx@...utronix.de>,
	Dave Hansen <dave.hansen@...ux.intel.com>,
	Jonathan Corbet <corbet@....net>, Ingo Molnar <mingo@...nel.org>,
	Pawan Gupta <pawan.kumar.gupta@...ux.intel.com>,
	Daniel Sneddon <daniel.sneddon@...ux.intel.com>,
	Kai Huang <kai.huang@...el.com>,
	Sandipan Das <sandipan.das@....com>,
	Breno Leitao <leitao@...ian.org>,
	Rick Edgecombe <rick.p.edgecombe@...el.com>,
	Alexei Starovoitov <ast@...nel.org>, Hou Tao <houtao1@...wei.com>,
	Juergen Gross <jgross@...e.com>,
	Vegard Nossum <vegard.nossum@...cle.com>,
	Eric Biggers <ebiggers@...gle.com>, Jason Gunthorpe <jgg@...pe.ca>,
	"Masami Hiramatsu (Google)" <mhiramat@...nel.org>,
	Andrew Morton <akpm@...ux-foundation.org>,
	Luis Chamberlain <mcgrof@...nel.org>,
	Yuntao Wang <ytcoode@...il.com>,
	Rasmus Villemoes <linux@...musvillemoes.dk>,
	Christophe Leroy <christophe.leroy@...roup.eu>,
	Tejun Heo <tj@...nel.org>, Changbin Du <changbin.du@...wei.com>,
	Huang Shijie <shijie@...amperecomputing.com>,
	Geert Uytterhoeven <geert+renesas@...der.be>,
	Namhyung Kim <namhyung@...nel.org>,
	Arnaldo Carvalho de Melo <acme@...hat.com>,
	linux-doc@...r.kernel.org, linux-kernel@...r.kernel.org,
	linux-efi@...r.kernel.org, linux-mm@...ck.org,
	"Kirill A. Shutemov" <kas@...nel.org>,
	"Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>,
	Andy Lutomirski <luto@...nel.org>, Ingo Molnar <mingo@...hat.com>,
	Borislav Petkov <bp@...en8.de>, "H. Peter Anvin" <hpa@...or.com>,
	Peter Zijlstra <peterz@...radead.org>,
	Ard Biesheuvel <ardb@...nel.org>,
	"Paul E. McKenney" <paulmck@...nel.org>,
	Josh Poimboeuf <jpoimboe@...nel.org>,
	Xiongwei Song <xiongwei.song@...driver.com>,
	Xin Li <xin3.li@...el.com>, "Mike Rapoport (IBM)" <rppt@...nel.org>,
	Brijesh Singh <brijesh.singh@....com>,
	Michael Roth <michael.roth@....com>,
	Tony Luck <tony.luck@...el.com>, Alexey Kardashevskiy <aik@....com>,
	Alexander Shishkin <alexander.shishkin@...ux.intel.com>,
	X86-kernel <x86@...nel.org>
Subject: Re: [PATCHv9 04/16] x86/cpu: Defer CR pinning setup until core
 initcall

On Thu, Jul 31, 2025 at 05:01:37PM -0700, Dave Hansen wrote:
> On 7/31/25 16:45, Sohil Mehta wrote:
> > On 7/9/2025 10:00 AM, Dave Hansen wrote:
> >> On 7/7/25 01:03, Kirill A. Shutemov wrote:
> >>> Instead of moving setup_cr_pinning() below efi_enter_virtual_mode() in
> >>> arch_cpu_finalize_init(), defer it until core initcall.
> >> What are the side effects of this move? Are there other benefits? What
> >> are the risks?
> >>
> > Picking this up from Kirill.. Reevaluating this, core_initcall() seems
> > too late for setup_cr_pinning().
> > 
> > We need to have CR pinning completed, and the associated static key
> > enabled before AP bring up. start_secondary()->cr4_init() depends on the
> > cr_pinning static key to initialize CR4 for APs.
> 
> Sure, if you leave cr4_init() completely as-is.
> 
> 'cr4_pinned_bits' should be set by the boot CPU. Secondary CPUs should
> also read 'cr4_pinned_bits' when setting up their own cr4's,
> unconditionally, independent of 'cr_pinning'.
> 
> The thing I think we should change is the pinning _enforcement_. The
> easiest way to do that is to remove the static_branch_likely() in
> cr4_init() and then delay flipping the static branch until just before
> userspace starts.

Yeah, this is fine from my perspective. The goal with the pinning was
about keeping things safe in the face of an attack from userspace that
managed to get at MSR values and keeping them from being trivially
changed.

-- 
Kees Cook