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

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: <3e46154d-6e5e-1c16-90fe-f2c5daa44b60@amd.com>
Date:   Mon, 13 Jan 2020 15:46:04 -0600
From:   Tom Lendacky <thomas.lendacky@....com>
To:     Arvind Sankar <nivedita@...m.mit.edu>,
        Thomas Gleixner <tglx@...utronix.de>,
        Ingo Molnar <mingo@...hat.com>, Borislav Petkov <bp@...en8.de>,
        "H. Peter Anvin" <hpa@...or.com>
Cc:     x86@...nel.org, linux-kernel@...r.kernel.org,
        Kees Cook <keescook@...omium.org>,
        Mauro Rossi <issor.oruam@...il.com>,
        Michael Matz <matz@...e.de>
Subject: Re: [PATCH v3] x86/vmlinux: Fix vmlinux.lds.S with pre-2.23 binutils

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@...m.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) {
>