linux-kernel - Re: [PATCH v2 18/39] x86/ibt: Add IBT feature, MSR and #CP handling

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: <Yh44YsW8viBscBD0@hirez.programming.kicks-ass.net>
Date:   Tue, 1 Mar 2022 16:14:42 +0100
From:   Peter Zijlstra <peterz@...radead.org>
To:     "Edgecombe, Rick P" <rick.p.edgecombe@...el.com>
Cc:     "Poimboe, Josh" <jpoimboe@...hat.com>,
        "hjl.tools@...il.com" <hjl.tools@...il.com>,
        "x86@...nel.org" <x86@...nel.org>,
        "joao@...rdrivepizza.com" <joao@...rdrivepizza.com>,
        "Cooper, Andrew" <andrew.cooper3@...rix.com>,
        "linux-kernel@...r.kernel.org" <linux-kernel@...r.kernel.org>,
        "keescook@...omium.org" <keescook@...omium.org>,
        "rostedt@...dmis.org" <rostedt@...dmis.org>,
        "samitolvanen@...gle.com" <samitolvanen@...gle.com>,
        "mark.rutland@....com" <mark.rutland@....com>,
        "alexei.starovoitov@...il.com" <alexei.starovoitov@...il.com>,
        "Milburn, Alyssa" <alyssa.milburn@...el.com>,
        "mhiramat@...nel.org" <mhiramat@...nel.org>,
        "mbenes@...e.cz" <mbenes@...e.cz>,
        "ndesaulniers@...gle.com" <ndesaulniers@...gle.com>
Subject: Re: [PATCH v2 18/39] x86/ibt: Add IBT feature, MSR and #CP handling

On Fri, Feb 25, 2022 at 07:59:15PM +0000, Edgecombe, Rick P wrote:
> On Thu, 2022-02-24 at 15:51 +0100, Peter Zijlstra wrote:
> > +__noendbr void cet_disable(void)
> > +{
> > +       if (cpu_feature_enabled(X86_FEATURE_IBT))
> > +               wrmsrl(MSR_IA32_S_CET, 0);
> > +}
> > +
> 
> Did this actually work? 

No idea,.. I don't generally have kexec clue.

> There are actually two problems with kexecing
> when CET is enabled. One is leaving the enforcement enabled when the
> new kernel can't handle it. The other is that CR4.CET and CR0.WP are
> tied together such that if you try to disable CR0.WP while CR4.CET is
> still set, it will #GP. CR0.WP gets unset during kexec/boot in the new
> kernel, so it blows up if you just disable IBT with the MSR and leave
> the CR4 bit set.
> 
> I was under the impression that this had been tested in the userspace
> series, but apparently not as I've just produced the CR0.WP issue. So
> it needs to be fixed in that series too. Userspace doesn't really need
> it pinned, so it should be easy.

So I see CR0 frobbing in identity_mapped and CR4 frobbing right after
it. Is there a reason to first do CR0 and then CR4 or can we flip them?
Otherwise we need to do CR4 twice.

(Also, whoever wrote that function with _5_ identically named labels in
it deserves something painful. Also, wth's up with that jmp 1f; 1:)

Something like so?

diff --git a/arch/x86/kernel/relocate_kernel_64.S b/arch/x86/kernel/relocate_kernel_64.S
index 399f075ccdc4..5b65f6ec5ee6 100644
--- a/arch/x86/kernel/relocate_kernel_64.S
+++ b/arch/x86/kernel/relocate_kernel_64.S
@@ -114,6 +114,14 @@ SYM_CODE_START_LOCAL_NOALIGN(identity_mapped)
 	/* store the start address on the stack */
 	pushq   %rdx
 
+	/*
+	 * Clear X86_CR4_CET (if it was set) such that we can clear CR0_WP
+	 * below.
+	 */
+	movq	%cr4, %rax
+	andq	$~(X86_CR4_CET), %rax
+	movq	%rax, %cr4
+
 	/*
 	 * Set cr0 to a known state:
 	 *  - Paging enabled