linux-kernel - Re: [PATCH v5 07/20] x86/kexec: Invoke copy of relocate

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: <20241216054942.GA25019@ax162>
Date: Sun, 15 Dec 2024 22:49:42 -0700
From: Nathan Chancellor <nathan@...nel.org>
To: David Woodhouse <dwmw2@...radead.org>
Cc: kexec@...ts.infradead.org, Thomas Gleixner <tglx@...utronix.de>,
	Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
	Dave Hansen <dave.hansen@...ux.intel.com>, x86@...nel.org,
	"H. Peter Anvin" <hpa@...or.com>,
	"Kirill A. Shutemov" <kirill.shutemov@...ux.intel.com>,
	Kai Huang <kai.huang@...el.com>,
	Nikolay Borisov <nik.borisov@...e.com>,
	linux-kernel@...r.kernel.org, Simon Horman <horms@...nel.org>,
	Dave Young <dyoung@...hat.com>,
	Peter Zijlstra <peterz@...radead.org>, jpoimboe@...nel.org,
	bsz@...zon.de
Subject: Re: [PATCH v5 07/20] x86/kexec: Invoke copy of relocate_kernel()
 instead of the original

On Sun, Dec 15, 2024 at 10:09:57AM +0000, David Woodhouse wrote:
> On Sat, 2024-12-14 at 16:08 -0700, Nathan Chancellor wrote:
> > 
> > I guess this seems somewhat unavoidable because control_page is just a
> > 'void *', perhaps machine_kexec() should just be marked as __nocfi? This
> > diff resolves that issue for me.
> 
> The patch below seems to work too. I already wanted to deal with the

Can confirm, thanks for the quick fix. With your fix for the first issue
I reported, the fix I sent for LTO, and this patch below, I can kexec on
a CFI and LTO enabled kernel without any issues.

> case where relocate_kernel isn't at the start of the page, so it forces
> me to do that.
> 
> For some reason it also started complaining
> vmlinux.o: warning: objtool: relocate_kernel+0x6a: return with modified stack frame
> ... which is easy to fix just by turning it into a jmp *%rsi; I have no
> idea why it was done with a ret like that in the first place.
> 
> I don't know why it puts 16 bytes of NOPs between __reloc_start and
> __cfi_relocate_kernel (in addition to the 16 before relocate_kernel
> itself), and space is *fairly* tight in the control page, but it's
> tolerable.

I think this is something to do with FineIBT IIRC? PeterZ might have
more details.

> To make the CFI check actually give useful output if it triggers, I'm
> tempted to do the IDT/GDT invalidation relocate_kernel itself, instead
> of before the call.
> 
> 
> diff --git a/arch/x86/kernel/relocate_kernel_64.S b/arch/x86/kernel/relocate_kernel_64.S
> index ccb297765e08..e6befd11fee4 100644
> --- a/arch/x86/kernel/relocate_kernel_64.S
> +++ b/arch/x86/kernel/relocate_kernel_64.S
> @@ -6,6 +6,7 @@
>  
>  #include <linux/linkage.h>
>  #include <linux/stringify.h>
> +#include <linux/cfi_types.h>
>  #include <asm/alternative.h>
>  #include <asm/page_types.h>
>  #include <asm/kexec.h>
> @@ -61,7 +62,10 @@ SYM_DATA_END(kexec_debug_idt)
>  
>  	.section .text.relocate_kernel,"ax";
>  	.code64
> -SYM_CODE_START_NOALIGN(relocate_kernel)
> +__reloc_start:
> +	ANNOTATE_NOENDBR
> +
> +SYM_TYPED_FUNC_START(relocate_kernel)
>  	UNWIND_HINT_END_OF_STACK
>  	ANNOTATE_NOENDBR
>  	/*
> @@ -115,10 +119,9 @@ SYM_CODE_START_NOALIGN(relocate_kernel)
>  	lea	PAGE_SIZE(%rsi), %rsp
>  
>  	/* jump to identity mapped page */
> -	addq	$(identity_mapped - relocate_kernel), %rsi
> -	pushq	%rsi
> -	ANNOTATE_UNRET_SAFE
> -	ret
> +	addq	$(identity_mapped - __reloc_start), %rsi
> +	ANNOTATE_RETPOLINE_SAFE
> +	jmp	*%rsi
>  	int3
>  SYM_CODE_END(relocate_kernel)
>  
> @@ -263,7 +266,7 @@ SYM_CODE_START_LOCAL_NOALIGN(identity_mapped)
>  
>  	/* get the re-entry point of the peer system */
>  	popq	%rbp
> -	leaq	relocate_kernel(%rip), %r8
> +	leaq	__reloc_start(%rip), %r8
>  	movq	kexec_pa_swap_page(%rip), %r10
>  	movq	pa_backup_pages_map(%rip), %rdi
>  	movq	kexec_pa_table_page(%rip), %rax
> @@ -272,7 +275,7 @@ SYM_CODE_START_LOCAL_NOALIGN(identity_mapped)
>  	movq	$1, %r11	/* Ensure preserve_context flag is set */
>  	call	swap_pages
>  	movq	kexec_va_control_page(%rip), %rax
> -	addq	$(virtual_mapped - relocate_kernel), %rax
> +	addq	$(virtual_mapped - __reloc_start), %rax
>  	pushq	%rax
>  	ANNOTATE_UNRET_SAFE
>  	ret
>