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(-)
From: Ard Biesheuvel <ardb@kernel.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@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Tiezhu Yang <yangtiezhu@loongson.cn>
Signed-off-by: Ard Biesheuvel <ardb@kernel.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"
struct special_alt {
struct list_head list;
--
2.48.1.601.g30ceb7b040-googOn Tue, Feb 18, 2025 at 10:25:39AM +0100, Ard Biesheuvel wrote:
> 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*) \
If I understand correctly, this is fixing an existing bug in loongarch
and any other arches using PIE, right? And it has nothing to do with
objtool?
If so, it feels like this needs to be its own patch, described as a fix.
> 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
Why have two different section names, does .data.rel.ro.* not work for
non-PIE?
--
Josh
On Tue, 18 Feb 2025 at 18:38, Josh Poimboeuf <jpoimboe@kernel.org> wrote:
>
> On Tue, Feb 18, 2025 at 10:25:39AM +0100, Ard Biesheuvel wrote:
> > 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*) \
>
> If I understand correctly, this is fixing an existing bug in loongarch
> and any other arches using PIE, right?
There are no other arches using PIE as far as I know. But it indeed
fixes an oversight in how -fPIE is used in the kernel.
> And it has nothing to do with
> objtool?
>
That didn't stop you from taking the previous fix :-)
> If so, it feels like this needs to be its own patch, described as a fix.
>
Fair enough. But better to drop the previous patch from the objtool tree then.
> > 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
>
> Why have two different section names, does .data.rel.ro.* not work for
> non-PIE?
>
Yeah, that is actually better, as long as we treat .data.rel.ro
consistently as .rodata.
On Tue, Feb 18, 2025 at 06:44:23PM +0100, Ard Biesheuvel wrote: > > If I understand correctly, this is fixing an existing bug in loongarch > > and any other arches using PIE, right? > > There are no other arches using PIE as far as I know. But it indeed > fixes an oversight in how -fPIE is used in the kernel. > > > And it has nothing to do with > > objtool? > > > > That didn't stop you from taking the previous fix :-) The whole point of the previous fix was to fix a bug in the objtool annotations. Unlike your patch, it didn't have any intended side effects. > > If so, it feels like this needs to be its own patch, described as a fix. > > > > Fair enough. But better to drop the previous patch from the objtool tree then. I think we can do that... Peter? And just to be clear, you'll have two fixes, right? 1) Make loongarch .data.rel.ro.* actually read-only 2) Fix objtool C jump table annotations for clang -- Josh
On Tue, Feb 18, 2025 at 10:31:08AM -0800, Josh Poimboeuf wrote: > > Fair enough. But better to drop the previous patch from the objtool tree then. > > I think we can do that... Peter? Let me see if git wants to co-operate :-) I think I managed to zap the top commit of tip/objtool/urgent.
On Tue, 18 Feb 2025 at 19:31, Josh Poimboeuf <jpoimboe@kernel.org> wrote: > > On Tue, Feb 18, 2025 at 06:44:23PM +0100, Ard Biesheuvel wrote: > > > If I understand correctly, this is fixing an existing bug in loongarch > > > and any other arches using PIE, right? > > > > There are no other arches using PIE as far as I know. But it indeed > > fixes an oversight in how -fPIE is used in the kernel. > > > > > And it has nothing to do with > > > objtool? > > > > > > > That didn't stop you from taking the previous fix :-) > > The whole point of the previous fix was to fix a bug in the objtool > annotations. Unlike your patch, it didn't have any intended side > effects. > Right, I skimmed over the missing ORC bit - I thought it was working around a linker warning. > > > If so, it feels like this needs to be its own patch, described as a fix. > > > > > > > Fair enough. But better to drop the previous patch from the objtool tree then. > > I think we can do that... Peter? > > And just to be clear, you'll have two fixes, right? > > 1) Make loongarch .data.rel.ro.* actually read-only > 2) Fix objtool C jump table annotations for clang > One fix is to emit .data.rel.ro input sections into .rodata, and fix objtool accordingly so it also looks for jump tables there. The other is to emit the BPF jump table into .data.rel.ro.c_jump_table so that the linker does not complain about conflicting permissions. Does that sound about right?
On Tue, Feb 18, 2025 at 08:36:42PM +0100, Ard Biesheuvel wrote:
> > > > If so, it feels like this needs to be its own patch, described as a fix.
> > > >
> > >
> > > Fair enough. But better to drop the previous patch from the objtool tree then.
> >
> > I think we can do that... Peter?
> >
> > And just to be clear, you'll have two fixes, right?
> >
> > 1) Make loongarch .data.rel.ro.* actually read-only
> > 2) Fix objtool C jump table annotations for clang
> >
>
> One fix is to emit .data.rel.ro input sections into .rodata, and fix
> objtool accordingly so it also looks for jump tables there.
Hm, I don't think so... I think at least one of us is confused :-)
Objtool reads object files (including vmlinux.o) before the final
vmlinux link. So a linker script change isn't visible to objtool.
And switch statment jump tables are always compiled in .rodata.*.
Right?
> The other is to emit the BPF jump table into .data.rel.ro.c_jump_table
> so that the linker does not complain about conflicting permissions.
>
> Does that sound about right?
I could be completely confused, but my understanding is we need the
following two fixes:
1) Map .data.rel.ro.* into .rodata for vmlinux to so that it actually
becomes read-only at runtime. This has nothing to do with objtool
AFAIK.
2) Fix the __annotate_jump_table macro for Clang, which takes:
__section(".rodata..c_jump_table,\"a\",@progbits #")
quite literally. So we need a new section name that works for GCC,
Clang, and GCC -fPIE. And it should be mapped in vmlinux .rodata.
--
Josh
On Tue, 18 Feb 2025 at 10:26, Ard Biesheuvel <ardb+git@google.com> wrote:
>
> From: Ard Biesheuvel <ardb@kernel.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@kernel.org>
> Cc: Peter Zijlstra <peterz@infradead.org>
> Cc: Tiezhu Yang <yangtiezhu@loongson.cn>
> Signed-off-by: Ard Biesheuvel <ardb@kernel.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.
On 02/18/2025 05:29 PM, Ard Biesheuvel wrote: > On Tue, 18 Feb 2025 at 10:26, Ard Biesheuvel <ardb+git@google.com> wrote: >> >> From: Ard Biesheuvel <ardb@kernel.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). ... > I'll send a fixed v2 once the LoongArch folks confirm that this works for them. This change works well with GCC and Clang on LoongArch. Tested-by: Tiezhu Yang <yangtiezhu@loongson.cn> # on LoongArch
© 2016 - 2025 Red Hat, Inc.