linux-kernel - Re: [PATCH] objtool: Use idiomatic section name for relocatable rodata under PIE

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: <CAMj1kXGyhYHS3WpC2oowWOnJiQVrbHOw57ULYuSx5Rip=Lt9oA@mail.gmail.com>
Date: Tue, 18 Feb 2025 10:29:07 +0100
From: Ard Biesheuvel <ardb@...nel.org>
To: Ard Biesheuvel <ardb+git@...gle.com>
Cc: linux-kernel@...r.kernel.org, x86@...nel.org, 
	Huacai Chen <chenhuacai@...nel.org>, Josh Poimboeuf <jpoimboe@...nel.org>, 
	Peter Zijlstra <peterz@...radead.org>, Tiezhu Yang <yangtiezhu@...ngson.cn>
Subject: Re: [PATCH] objtool: Use idiomatic section name for relocatable
 rodata under PIE

On Tue, 18 Feb 2025 at 10:26, Ard Biesheuvel <ardb+git@...gle.com> wrote:
>
> From: Ard Biesheuvel <ardb@...nel.org>
>
> When running in PIE mode, the compiler will emit const global objects
> into .data.rel.ro rather than into .rodata if those objects contain
> statically initialized fields carrying addresses that are subject to
> runtime relocation (e.g., function pointers).
>
> This is needed so that the user space runtime linker can identify which
> parts of the executable image need to be writable initially, but can be
> converted into read-only before the image starts executing.
>
> This distinction does not matter for the kernel, but when using the
> compiler in PIE mode (such as when building for LoongArch), those
> .data.rel.ro sections need to be treated as .rodata as well.
>
> It also means that manually placed const global objects that contain
> absolute addresses (such as the non-JIT BPF jump table) need to be
> emitted into .data.rel.ro too so that the linker does not complain about
> conflicting permissions.
>
> Cc: Josh Poimboeuf <jpoimboe@...nel.org>
> Cc: Peter Zijlstra <peterz@...radead.org>
> Cc: Tiezhu Yang <yangtiezhu@...ngson.cn>
> Signed-off-by: Ard Biesheuvel <ardb@...nel.org>
> ---
> Please consider this approach instead of the ..rodata hack - thanks.
>
>  include/asm-generic/vmlinux.lds.h       | 2 +-
>  include/linux/compiler.h                | 6 +++++-
>  tools/objtool/check.c                   | 7 +++----
>  tools/objtool/include/objtool/special.h | 2 +-
>  4 files changed, 10 insertions(+), 7 deletions(-)
>
> diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
> index 91a7e824ed8b..337d3336e175 100644
> --- a/include/asm-generic/vmlinux.lds.h
> +++ b/include/asm-generic/vmlinux.lds.h
> @@ -457,7 +457,7 @@ defined(CONFIG_AUTOFDO_CLANG) || defined(CONFIG_PROPELLER_CLANG)
>         . = ALIGN((align));                                             \
>         .rodata           : AT(ADDR(.rodata) - LOAD_OFFSET) {           \
>                 __start_rodata = .;                                     \
> -               *(.rodata) *(.rodata.*) *(..rodata.*)                   \
> +               *(.rodata) *(.rodata.*) *(.data.rel.ro*)                \
>                 SCHED_DATA                                              \
>                 RO_AFTER_INIT_DATA      /* Read only after init */      \
>                 . = ALIGN(8);                                           \
> diff --git a/include/linux/compiler.h b/include/linux/compiler.h
> index 3d013f1412e0..27024a128a6a 100644
> --- a/include/linux/compiler.h
> +++ b/include/linux/compiler.h
> @@ -110,7 +110,11 @@ void ftrace_likely_update(struct ftrace_likely_data *f, int val,
>  /* Unreachable code */
>  #ifdef CONFIG_OBJTOOL
>  /* Annotate a C jump table to allow objtool to follow the code flow */
> -#define __annotate_jump_table __section("..rodata.c_jump_table")
> +#ifndef __pie__
> +#define __annotate_jump_table __section(".rodata.c_jump_table")
> +#else
> +#define __annotate_jump_table __section(".data.rel.ro.c_jump_table")
> +#endif
>  #else /* !CONFIG_OBJTOOL */
>  #define __annotate_jump_table
>  #endif /* CONFIG_OBJTOOL */
> diff --git a/tools/objtool/check.c b/tools/objtool/check.c
> index 1398ffc20b16..898d0cee4565 100644
> --- a/tools/objtool/check.c
> +++ b/tools/objtool/check.c
> @@ -2471,14 +2471,13 @@ static void mark_rodata(struct objtool_file *file)
>          *
>          * - .rodata: can contain GCC switch tables
>          * - .rodata.<func>: same, if -fdata-sections is being used
> -        * - ..rodata.c_jump_table: contains C annotated jump tables
> +        * - .data.rel.ro: same when using -fPIE codegen
>          *
>          * .rodata.str1.* sections are ignored; they don't contain jump tables.
>          */
>         for_each_sec(file, sec) {
> -               if ((!strncmp(sec->name, ".rodata", 7) ||
> -                   !strncmp(sec->name, "..rodata", 8)) &&
> -                   !strstr(sec->name, ".str1.")) {
> +               if ((!strncmp(sec->name, ".rodata", 7) && !strstr(sec->name, ".str1.")) ||
> +                   !strncmp(sec->name, ".data.rel.ro", 12)) {
>                         sec->rodata = true;
>                         found = true;
>                 }
> diff --git a/tools/objtool/include/objtool/special.h b/tools/objtool/include/objtool/special.h
> index 34acf4ae5fab..e049679bb17b 100644
> --- a/tools/objtool/include/objtool/special.h
> +++ b/tools/objtool/include/objtool/special.h
> @@ -10,7 +10,7 @@
>  #include <objtool/check.h>
>  #include <objtool/elf.h>
>
> -#define C_JUMP_TABLE_SECTION "..rodata.c_jump_table"
> +#define C_JUMP_TABLE_SECTION ".data.rel.ro.c_jump_table"
>

Ugh - just spotted myself that this needs a #ifndef __pie__ too (or we
just emit into .data.rel.ro unconditionally, which would also be
fine).

I'll send a fixed v2 once the LoongArch folks confirm that this works for them.