Messages in this thread Patch in this message | | | Date | Tue, 1 Mar 2022 16:14:42 +0100 | From | Peter Zijlstra <> | 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
| |