linux-kernel - Re: [PATCH 1/2] x86/retpoline,kprobes: Fix position of thunk sections with CONFIG_LTO

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: <20230705085222.GF462772@hirez.programming.kicks-ass.net>
Date:   Wed, 5 Jul 2023 10:52:22 +0200
From:   Peter Zijlstra <peterz@...radead.org>
To:     Petr Pavlu <petr.pavlu@...e.com>
Cc:     tglx@...utronix.de, mingo@...hat.com, bp@...en8.de,
        dave.hansen@...ux.intel.com, hpa@...or.com, mhiramat@...nel.org,
        samitolvanen@...gle.com, x86@...nel.org,
        linux-trace-kernel@...r.kernel.org, linux-kernel@...r.kernel.org
Subject: Re: [PATCH 1/2] x86/retpoline,kprobes: Fix position of thunk
 sections with CONFIG_LTO_CLANG

On Wed, Jul 05, 2023 at 10:15:46AM +0200, Petr Pavlu wrote:
> Linker script arch/x86/kernel/vmlinux.lds.S matches the thunk sections
> ".text.__x86.*" from arch/x86/lib/retpoline.S as follows:
> 
> .text {
>   [...]
>   TEXT_TEXT
>   [...]
>   __indirect_thunk_start = .;
>   *(.text.__x86.*)
>   __indirect_thunk_end = .;
>   [...]
> }
> 
> Macro TEXT_TEXT references TEXT_MAIN which normally expands to only
> ".text". However, with CONFIG_LTO_CLANG, TEXT_MAIN becomes
> ".text .text.[0-9a-zA-Z_]*" which wrongly matches also the thunk
> sections. The range [__indirect_thunk_start, __indirect_thunk_end] is
> then empty.
> 
> A visible result is that function insn_is_indirect_jump() misbehaves and
> optprobes become allowed in functions where they are not potentially
> safe.
> 
> Fix the problem by using ".." as the first separator, for instance,
> ".text..__x86.indirect_thunk". This pattern is utilized by other

First I hear of this, but yes, indeed.

> explicit section names which start with one of the standard prefixes,
> such as ".text" or ".data", and that need to be individually selected in
> the linker script.
> 
> Fixes: dc5723b02e52 ("kbuild: add support for Clang LTO")
> Signed-off-by: Petr Pavlu <petr.pavlu@...e.com>

Acked-by: Peter Zijlstra (Intel) <peterz@...radead.org>

> ---
>  arch/x86/kernel/vmlinux.lds.S | 2 +-
>  arch/x86/lib/retpoline.S      | 4 ++--
>  2 files changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/arch/x86/kernel/vmlinux.lds.S b/arch/x86/kernel/vmlinux.lds.S
> index 03c885d3640f..a4cd04c458df 100644
> --- a/arch/x86/kernel/vmlinux.lds.S
> +++ b/arch/x86/kernel/vmlinux.lds.S
> @@ -134,7 +134,7 @@ SECTIONS
>  		SOFTIRQENTRY_TEXT
>  #ifdef CONFIG_RETPOLINE
>  		__indirect_thunk_start = .;
> -		*(.text.__x86.*)
> +		*(.text..__x86.*)
>  		__indirect_thunk_end = .;
>  #endif
>  		STATIC_CALL_TEXT
> diff --git a/arch/x86/lib/retpoline.S b/arch/x86/lib/retpoline.S
> index 3fd066d42ec0..3bea96341d00 100644
> --- a/arch/x86/lib/retpoline.S
> +++ b/arch/x86/lib/retpoline.S
> @@ -12,7 +12,7 @@
>  #include <asm/percpu.h>
>  #include <asm/frame.h>
>  
> -	.section .text.__x86.indirect_thunk
> +	.section .text..__x86.indirect_thunk
>  
>  
>  .macro POLINE reg
> @@ -131,7 +131,7 @@ SYM_CODE_END(__x86_indirect_jump_thunk_array)
>   */
>  #ifdef CONFIG_RETHUNK
>  
> -	.section .text.__x86.return_thunk
> +	.section .text..__x86.return_thunk
>  
>  /*
>   * Safety details here pertain to the AMD Zen{1,2} microarchitecture:
> -- 
> 2.35.3
>