[PATCH v2 0/2] Handle .data.rel.ro correctly and use it for BPF

[PATCH v2 0/2] Handle .data.rel.ro correctly and use it for BPF

[Ard Biesheuvel]

From: Ard Biesheuvel <ardb@kernel.org>

LoongArch uses PIE codegen for the entire kernel, which has some
implications for how .rodata is populated and emitted into vmlinux.

Changes since v1: [0]

- Always use .data.rel.ro.c_jump_table for C jump tables
- Add Tiezhu Yang's Tested-by
- Split into two patches and rebase onto Linus's tree

[0] https://lore.kernel.org/all/20250218092538.1903204-2-ardb+git@google.com/T/#u

Cc: Josh Poimboeuf <jpoimboe@kernel.org>
Cc: Peter Zijlstra <peterz@infradead.org>
Cc: Tiezhu Yang <yangtiezhu@loongson.cn>

Ard Biesheuvel (2):
  asm-generic/vmlinux.lds: Move .data.rel.ro input into .rodata segment
  objtool: Use fPIE compatible ELF sections for C jump tables

 include/asm-generic/vmlinux.lds.h       | 2 +-
 include/linux/compiler.h                | 2 +-
 tools/objtool/check.c                   | 7 ++++---
 tools/objtool/include/objtool/special.h | 2 +-
 4 files changed, 7 insertions(+), 6 deletions(-)

-- 
2.48.1.601.g30ceb7b040-goog

[PATCH v2 1/2] asm-generic/vmlinux.lds: Move .data.rel.ro input into .rodata segment

[Ard Biesheuvel]

From: Ard Biesheuvel <ardb@kernel.org>

When using -fPIE codegen, the compiler will emit const global objects
(which are useless unless statically initialized) into .data.rel.ro
rather than .rodata if the object contains fields that carry absolute
addresses of other code or data objects. This permits the linker to
annotate such regions as requiring read-write access only at load time,
but not at execution time (in user space).

This distinction does not matter for the kernel, but it does imply that
const data will end up in writable memory if the .data.rel.ro sections
are not treated in a special way.

So emit .data.rel.ro into the .rodata segment.

Cc: <stable@vger.kernel.org>
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 include/asm-generic/vmlinux.lds.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/include/asm-generic/vmlinux.lds.h b/include/asm-generic/vmlinux.lds.h
index 02a4adb4a999..0d5b186abee8 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.*) *(.data.rel.ro*)		\
 		SCHED_DATA						\
 		RO_AFTER_INIT_DATA	/* Read only after init */	\
 		. = ALIGN(8);						\
-- 
2.48.1.601.g30ceb7b040-goog

Re: [PATCH v2 1/2] asm-generic/vmlinux.lds: Move .data.rel.ro input into .rodata segment

[Josh Poimboeuf]

On Wed, Feb 19, 2025 at 11:55:44AM +0100, Ard Biesheuvel wrote:
> From: Ard Biesheuvel <ardb@kernel.org>

BTW, I was copied on the cover letter but not the individual patches.

> When using -fPIE codegen, the compiler will emit const global objects
> (which are useless unless statically initialized) into .data.rel.ro
> rather than .rodata if the object contains fields that carry absolute
> addresses of other code or data objects. This permits the linker to
> annotate such regions as requiring read-write access only at load time,
> but not at execution time (in user space).

Hm, this sounds more like __ro_after_init, are we sure the kernel
doesn't need to write it early?

> This distinction does not matter for the kernel, but it does imply that
> const data will end up in writable memory if the .data.rel.ro sections
> are not treated in a special way.
> 
> So emit .data.rel.ro into the .rodata segment.

This is a bug fix, right?  Since the RO data wasn't actually RO?  That's
not clear in the title.

-- 
Josh

Re: [PATCH v2 1/2] asm-generic/vmlinux.lds: Move .data.rel.ro input into .rodata segment

[Ard Biesheuvel]

On Thu, 20 Feb 2025 at 21:55, Josh Poimboeuf <jpoimboe@kernel.org> wrote:
>
> On Wed, Feb 19, 2025 at 11:55:44AM +0100, Ard Biesheuvel wrote:
> > From: Ard Biesheuvel <ardb@kernel.org>
>
> BTW, I was copied on the cover letter but not the individual patches.
>
> > When using -fPIE codegen, the compiler will emit const global objects
> > (which are useless unless statically initialized) into .data.rel.ro
> > rather than .rodata if the object contains fields that carry absolute
> > addresses of other code or data objects. This permits the linker to
> > annotate such regions as requiring read-write access only at load time,
> > but not at execution time (in user space).
>
> Hm, this sounds more like __ro_after_init, are we sure the kernel
> doesn't need to write it early?
>

It does need to write to it early, for KASLR relocation. So
conceptually, it is the same as .rodata. __ro_after_init conceptually
remains writable for longer.

In practice, they are all treated the same so it doesn't really matter.

> > This distinction does not matter for the kernel, but it does imply that
> > const data will end up in writable memory if the .data.rel.ro sections
> > are not treated in a special way.
> >
> > So emit .data.rel.ro into the .rodata segment.
>
> This is a bug fix, right?  Since the RO data wasn't actually RO?  That's
> not clear in the title.
>

Yes, it is. I'll clarify that.

[PATCH v2 2/2] objtool: Use fPIE compatible ELF sections for C jump tables

[Ard Biesheuvel]

From: Ard Biesheuvel <ardb@kernel.org>

A C jump table (such as the one used by the BPF interpreter) is a const
global array of absolute code addresses, and this means that the actual
values in the table may not be known until the kernel is booted (e.g.,
when using KASLR or when the kernel VA space is sized dynamically).

When using -fPIE codegen, const global objects of this nature will
generally be placed in .data.rel.ro rather than .rodata by the compiler,
and forcing these C jump tables into .rodata like is done currently will
trigger warnings from the linker about combining read-only and
read-write input sections into the same output section.

Avoid such warnings by unconditionally emitting C jump tables into
.data.rel.ro, which will always be placed appropriately regardless of
whether -fPIE is actually being used.

Note that, while possible in theory, compiler generated jump tables are
unlikely to end up in .data.rel.ro, as the compiler will use relative
references when using -fPIE, and these can be resolved at build time.

This supersedes commit

  c5b1184decc8 ("compiler.h: specify correct attribute for .rodata..c_jump_table")

which addressed the linker warnings by injecting section attributes into
the __attribute__((section(""))) name string, but this turns out not to
work reliably across toolchains, and may result in missing ORC data in
some cases.

Fixes: c5b1184decc8 ("compiler.h: specify correct attribute for .rodata..c_jump_table")
Tested-by: Tiezhu Yang <yangtiezhu@loongson.cn> # on LoongArch
Signed-off-by: Ard Biesheuvel <ardb@kernel.org>
---
 include/linux/compiler.h                | 2 +-
 tools/objtool/check.c                   | 7 ++++---
 tools/objtool/include/objtool/special.h | 2 +-
 3 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/include/linux/compiler.h b/include/linux/compiler.h
index 200fd3c5bc70..155385754824 100644
--- a/include/linux/compiler.h
+++ b/include/linux/compiler.h
@@ -110,7 +110,7 @@ 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,\"a\",@progbits #")
+#define __annotate_jump_table __section(".data.rel.ro.c_jump_table")
 #else /* !CONFIG_OBJTOOL */
 #define __annotate_jump_table
 #endif /* CONFIG_OBJTOOL */
diff --git a/tools/objtool/check.c b/tools/objtool/check.c
index be18a0489303..ce973d9d8e6d 100644
--- a/tools/objtool/check.c
+++ b/tools/objtool/check.c
@@ -2472,13 +2472,14 @@ 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.c_jump_table: contains C annotated jump tables
 	 *
 	 * .rodata.str1.* sections are ignored; they don't contain jump tables.
 	 */
 	for_each_sec(file, sec) {
-		if (!strncmp(sec->name, ".rodata", 7) &&
-		    !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 e7ee7ffccefd..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-goog

Re: [PATCH v2 2/2] objtool: Use fPIE compatible ELF sections for C jump tables

[Josh Poimboeuf]

On Wed, Feb 19, 2025 at 11:55:45AM +0100, Ard Biesheuvel wrote:
> From: Ard Biesheuvel <ardb@kernel.org>
> 
> A C jump table (such as the one used by the BPF interpreter) is a const
> global array of absolute code addresses, and this means that the actual
> values in the table may not be known until the kernel is booted (e.g.,
> when using KASLR or when the kernel VA space is sized dynamically).
> 
> When using -fPIE codegen, const global objects of this nature will
> generally be placed in .data.rel.ro rather than .rodata by the compiler,
> and forcing these C jump tables into .rodata like is done currently will
> trigger warnings from the linker about combining read-only and
> read-write input sections into the same output section.
>
> Avoid such warnings by unconditionally emitting C jump tables into
> .data.rel.ro, which will always be placed appropriately regardless of
> whether -fPIE is actually being used.

Just to clarify, currently there are no linker warnings.  The original
linker warnings (fixed by c5b1184decc8) were only with the GNU
toolchain, and only on LoongArch.  Clang/LLVM apparently didn't care.

The only problem being fixed here is that the hack in c5b1184decc8
doesn't work with Clang/LLVM, so the C jump tables get placed in
'.rodata..c_jump_table,"a",@progbits #'.  Which confuses objtool,
spitting out the following warning:

     kernel/bpf/core.o: warning: objtool: ___bpf_prog_run+0x20: sibling call from callable instruction with modified stack frame

> Note that, while possible in theory, compiler generated jump tables are
> unlikely to end up in .data.rel.ro, as the compiler will use relative
> references when using -fPIE, and these can be resolved at build time.
> 
> This supersedes commit
> 
>   c5b1184decc8 ("compiler.h: specify correct attribute for .rodata..c_jump_table")
> 
> which addressed the linker warnings by injecting section attributes into
> the __attribute__((section(""))) name string, but this turns out not to
> work reliably across toolchains, and may result in missing ORC data in
> some cases.

Missing ORC data was a side effect, it's really the warning which should
be called out in the description.

Also, since this is indeed a bug fix, that should be made clear in the
subject, something like:

  objtool: Fix C jump table annotations for Clang

> Fixes: c5b1184decc8 ("compiler.h: specify correct attribute for .rodata..c_jump_table")

Reported-by: https://lore.kernel.org/202501141306.GYbbghj5-lkp@intel.com

-- 
Josh

Re: [PATCH v2 2/2] objtool: Use fPIE compatible ELF sections for C jump tables

[Josh Poimboeuf]

On Thu, Feb 20, 2025 at 01:25:17PM -0800, Josh Poimboeuf wrote:
> > Fixes: c5b1184decc8 ("compiler.h: specify correct attribute for .rodata..c_jump_table")
> 
> Reported-by: https://lore.kernel.org/202501141306.GYbbghj5-lkp@intel.com

Or rather:

Reported-by: kernel test robot <lkp@intel.com>
Closes: https://lore.kernel.org/r/202501141306.GYbbghj5-lkp@intel.com/

-- 
Josh

Re: [PATCH v2 2/2] objtool: Use fPIE compatible ELF sections for C jump tables

[Ard Biesheuvel]

On Thu, 20 Feb 2025 at 22:27, Josh Poimboeuf <jpoimboe@kernel.org> wrote:
>
> On Thu, Feb 20, 2025 at 01:25:17PM -0800, Josh Poimboeuf wrote:
> > > Fixes: c5b1184decc8 ("compiler.h: specify correct attribute for .rodata..c_jump_table")
> >
> > Reported-by: https://lore.kernel.org/202501141306.GYbbghj5-lkp@intel.com
>
> Or rather:
>
> Reported-by: kernel test robot <lkp@intel.com>
> Closes: https://lore.kernel.org/r/202501141306.GYbbghj5-lkp@intel.com/
>

OK i'll fix that up.