Re: [PATCH v3] x86/vmlinux: Fix vmlinux.lds.S with pre-2.23 binutils

From: Tom Lendacky <thomas.lendacky@amd.com>
To: Arvind Sankar <nivedita@alum.mit.edu>,
	Thomas Gleixner <tglx@linutronix.de>,
	Ingo Molnar <mingo@redhat.com>, Borislav Petkov <bp@alien8.de>,
	"H. Peter Anvin" <hpa@zytor.com>
Cc: x86@kernel.org, linux-kernel@vger.kernel.org,
	Kees Cook <keescook@chromium.org>,
	Mauro Rossi <issor.oruam@gmail.com>, Michael Matz <matz@suse.de>
Subject: Re: [PATCH v3] x86/vmlinux: Fix vmlinux.lds.S with pre-2.23 binutils
Date: Mon, 13 Jan 2020 15:46:04 -0600	[thread overview]
Message-ID: <3e46154d-6e5e-1c16-90fe-f2c5daa44b60@amd.com> (raw)
In-Reply-To: <20200113195337.604646-1-nivedita@alum.mit.edu>

On 1/13/20 1:53 PM, Arvind Sankar wrote:
> Prior to binutils-2.23, ld treats the location counter as absolute if
> used outside an output section definition. From version 2.23 onwards,
> the location counter is treated as relative to an adjacent output
> section (usually the previous one, unless there isn't one or the
> location counter has been assigned to previously, in which case the next
> one).
> 
> The result is that a symbol definition in the linker script, such as
> 	_etext = .;
> that appears outside an output section definition makes _etext an
> absolute symbol prior to binutils-2.23 and a relative symbol from
> version 2.23 onwards. So when using a 2.21 or 2.22 vintage linker, the
> build fails with
> 	Invalid absolute R_X86_64_32S relocation: _etext
> for x86-64, and a similar message with R_386_32 for x86-32.
> 
> This can be reproduced with the official 2.21.1 and 2.22 binutils
> releases.
> 
> Commit b907693883fd ("x86/vmlinux: Actually use _etext for the end of
> the text segment") moved _etext out of the .text section to place it
> after the exception table, however since commit f0d7ee17d57c
> ("x86/vmlinux: Move EXCEPTION_TABLE to RO_DATA segment") this is no
> longer needed. Move _etext back inside .text to make it relative even
> with older linkers.
> 
> Commit c603a309cc75 ("x86/mm: Identify the end of the kernel area to be
> reserved") defines __end_of_kernel_reserve using the location counter
> outside an output section definition. Use __bss_stop instead of the
> location counter for the definition to make it relative with older
> linkers.
> 
> Fixes: b907693883fd ("x86/vmlinux: Actually use _etext for the end of the text segment")
> Fixes: c603a309cc75 ("x86/mm: Identify the end of the kernel area to be reserved")
> Signed-off-by: Arvind Sankar <nivedita@alum.mit.edu>
> ---
> v3: Modify vmlinux.lds.S instead of adding more workarounds to tools/relocs.c
> 
>  arch/x86/kernel/vmlinux.lds.S | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/x86/kernel/vmlinux.lds.S b/arch/x86/kernel/vmlinux.lds.S
> index 3a1a819da137..bad4e22384dc 100644
> --- a/arch/x86/kernel/vmlinux.lds.S
> +++ b/arch/x86/kernel/vmlinux.lds.S
> @@ -144,10 +144,12 @@ SECTIONS
>  		*(.text.__x86.indirect_thunk)
>  		__indirect_thunk_end = .;
>  #endif
> +
> +		/* End of text section */
> +		_etext = .;
>  	} :text =0xcccc
>  
> -	/* End of text section, which should occupy whole number of pages */
> -	_etext = .;
> +	/* .text should occupy whole number of pages */
>  	. = ALIGN(PAGE_SIZE);
>  
>  	X86_ALIGN_RODATA_BEGIN
> @@ -372,7 +374,7 @@ SECTIONS
>  	 * explicitly reserved using memblock_reserve() or it will be discarded
>  	 * and treated as available memory.
>  	 */
> -	__end_of_kernel_reserve = .;
> +	__end_of_kernel_reserve = __bss_stop;

The only thing I worry about is if someone inserts a section between
the bss section and here and doesn't update this value.

Thanks,
Tom

>  
>  	. = ALIGN(PAGE_SIZE);
>  	.brk : AT(ADDR(.brk) - LOAD_OFFSET) {
> 