[PATCH 0/6] s390: Add kCFI support

[PATCH 0/6] s390: Add kCFI support

[Heiko Carstens]

Add s390 kCFI support using the generic support provided by clang.
This comes with a couple of limitations:

The generic kCFI implementation does not generate a .kcfi_traps section,
nor is a special instruction used in case a checksum mismatch is detected.

This means in case of a checksum mismatch the kernel just crashes.
It should be quite easy to tell by the surrounding code that a crash
happened because of a checksum mismatch.

If clang and/or gcc provide a .kcfi_traps section it will be possible to
print proper CFI messages instead of just crashing the kernel (enable
ARCH_USES_CFI_TRAPS).

In addition this also means that CFI_PERMISSIVE does not work. Even if the
option is selected the kernel will crash in case of a checksum mismatch.

However it seems to be acceptable to enable kCFI support to the kernel now
even if it is not perfect. Later s390 specific clang and gcc extensions are
required to improve this.

This passes all ftrace selftests. It also passes the kernel bpf selftests
(vmtest.sh), except those using the task work scheduling kfuncs. However
that appears to be a real bug and has been reported [1].

[1] https://lore.kernel.org/all/20260724112221.212464A3f-hca@linux.ibm.com/

Thanks,
Heiko

Heiko Carstens (6):
  s390/tools: Pass symbol name to do_relocs()
  s390/tools/relocs: Ignore __kcfi_typeid_ relocations
  s390: Add ftrace_stub_graph
  s390/diag: Generate CFI type information for assembly functions
  s390/Kconfig: Select ARCH_SUPPORTS_CFI
  s390/bpf: Add kCFI support

 arch/s390/Kconfig               |   1 +
 arch/s390/include/asm/cfi.h     |   7 ++
 arch/s390/kernel/mcount.S       |   7 +-
 arch/s390/kernel/text_amode31.S |  13 +--
 arch/s390/net/bpf_jit_comp.c    |  26 +++++-
 arch/s390/tools/relocs.c        | 158 +++++++++++++++++++++++++++++++-
 6 files changed, 198 insertions(+), 14 deletions(-)
 create mode 100644 arch/s390/include/asm/cfi.h

-- 
2.53.0

[PATCH 1/6] s390/tools: Pass symbol name to do_relocs()

[Heiko Carstens]

Pass the symbol symbol name which corresponds to a relocation to
do_relocs(). This is preparation for kcfi support.

Given that the s390 specific relocs tool is a stripped down version of the
x86 version, add more code from the x86 version to the s390 version, while
keeping coding style, etc. in order to add the required functionality.

Reviewed-by: Jens Remus <jremus@linux.ibm.com>
Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
---
 arch/s390/tools/relocs.c | 142 ++++++++++++++++++++++++++++++++++++++-
 1 file changed, 139 insertions(+), 3 deletions(-)

diff --git a/arch/s390/tools/relocs.c b/arch/s390/tools/relocs.c
index 30a732c808f3..a2774176d38f 100644
--- a/arch/s390/tools/relocs.c
+++ b/arch/s390/tools/relocs.c
@@ -41,6 +41,10 @@
 static Elf_Ehdr		ehdr;
 static unsigned long	shnum;
 static unsigned int	shstrndx;
+static unsigned int	shsymtabndx;
+static unsigned int	shxsymtabndx;
+
+static int sym_index(Elf_Sym *sym);
 
 struct relocs {
 	uint32_t	*offset;
@@ -54,11 +58,40 @@ static struct relocs relocs64;
 struct section {
 	Elf_Shdr	shdr;
 	struct section	*link;
+	Elf_Sym		*symtab;
+	Elf32_Word	*xsymtab;
 	Elf_Rel		*reltab;
+	char		*strtab;
 };
 
 static struct section *secs;
 
+static const char *sec_name(unsigned shndx)
+{
+	const char *sec_strtab;
+	const char *name = "<noname>";
+	sec_strtab = secs[shstrndx].strtab;
+
+	if (shndx < shnum)
+		name = sec_strtab + secs[shndx].shdr.sh_name;
+	else if (shndx == SHN_ABS)
+		name = "ABSOLUTE";
+	else if (shndx == SHN_COMMON)
+		name = "COMMON";
+	return name;
+}
+
+static const char *sym_name(const char *sym_strtab, Elf_Sym *sym)
+{
+	const char *name;
+
+	if (sym->st_name)
+		name = sym_strtab + sym->st_name;
+	else
+		name = sec_name(sym_index(sym));
+	return name;
+}
+
 #if BYTE_ORDER == LITTLE_ENDIAN
 #define le16_to_cpu(val)	(val)
 #define le32_to_cpu(val)	(val)
@@ -105,6 +138,23 @@ static uint64_t elf64_to_cpu(uint64_t val)
 #define elf_off_to_cpu(x)	elf64_to_cpu(x)
 #define elf_xword_to_cpu(x)	elf64_to_cpu(x)
 
+static int sym_index(Elf_Sym *sym)
+{
+	Elf_Sym *symtab = secs[shsymtabndx].symtab;
+	Elf32_Word *xsymtab = secs[shxsymtabndx].xsymtab;
+	unsigned long offset;
+	int index;
+
+	if (sym->st_shndx != SHN_XINDEX)
+		return sym->st_shndx;
+
+	/* calculate offset of sym from head of table. */
+	offset = (unsigned long)sym - (unsigned long)symtab;
+	index = offset / sizeof(*sym);
+
+	return elf32_to_cpu(xsymtab[index]);
+}
+
 static void die(char *fmt, ...)
 {
 	va_list ap;
@@ -216,6 +266,81 @@ static void read_shdrs(FILE *fp)
 
 }
 
+static void read_strtabs(FILE *fp)
+{
+	int i;
+
+	for (i = 0; i < shnum; i++) {
+		struct section *sec = &secs[i];
+
+		if (sec->shdr.sh_type != SHT_STRTAB)
+			continue;
+
+		sec->strtab = malloc(sec->shdr.sh_size);
+		if (!sec->strtab)
+			die("malloc of %" FMT " bytes for strtab failed\n", sec->shdr.sh_size);
+
+		if (fseek(fp, sec->shdr.sh_offset, SEEK_SET) < 0)
+			die("Seek to %" FMT " failed: %s\n", sec->shdr.sh_offset, strerror(errno));
+
+		if (fread(sec->strtab, 1, sec->shdr.sh_size, fp) != sec->shdr.sh_size)
+			die("Cannot read symbol table: %s\n", strerror(errno));
+	}
+}
+
+static void read_symtabs(FILE *fp)
+{
+	int i, j;
+
+	for (i = 0; i < shnum; i++) {
+		struct section *sec = &secs[i];
+		int num_syms;
+
+		switch (sec->shdr.sh_type) {
+		case SHT_SYMTAB_SHNDX:
+			sec->xsymtab = malloc(sec->shdr.sh_size);
+			if (!sec->xsymtab)
+				die("malloc of %" FMT " bytes for xsymtab failed\n", sec->shdr.sh_size);
+
+			if (fseek(fp, sec->shdr.sh_offset, SEEK_SET) < 0)
+				die("Seek to %" FMT " failed: %s\n", sec->shdr.sh_offset, strerror(errno));
+
+			if (fread(sec->xsymtab, 1, sec->shdr.sh_size, fp) != sec->shdr.sh_size)
+				die("Cannot read extended symbol table: %s\n", strerror(errno));
+
+			shxsymtabndx = i;
+			continue;
+
+		case SHT_SYMTAB:
+			num_syms = sec->shdr.sh_size / sizeof(Elf_Sym);
+
+			sec->symtab = malloc(sec->shdr.sh_size);
+			if (!sec->symtab)
+				die("malloc of %" FMT " bytes for symtab failed\n", sec->shdr.sh_size);
+
+			if (fseek(fp, sec->shdr.sh_offset, SEEK_SET) < 0)
+				die("Seek to %" FMT " failed: %s\n", sec->shdr.sh_offset, strerror(errno));
+
+			if (fread(sec->symtab, 1, sec->shdr.sh_size, fp) != sec->shdr.sh_size)
+				die("Cannot read symbol table: %s\n", strerror(errno));
+
+			for (j = 0; j < num_syms; j++) {
+				Elf_Sym *sym = &sec->symtab[j];
+
+				sym->st_name  = elf_word_to_cpu(sym->st_name);
+				sym->st_value = elf_addr_to_cpu(sym->st_value);
+				sym->st_size  = elf_xword_to_cpu(sym->st_size);
+				sym->st_shndx = elf_half_to_cpu(sym->st_shndx);
+			}
+			shsymtabndx = i;
+			continue;
+
+		default:
+			continue;
+		}
+	}
+}
+
 static void read_relocs(FILE *fp)
 {
 	int i, j;
@@ -263,7 +388,8 @@ static void add_reloc(struct relocs *r, uint32_t offset)
 	r->offset[r->count++] = offset;
 }
 
-static int do_reloc(struct section *sec, Elf_Rel *rel)
+static int do_reloc(struct section *sec, Elf_Rel *rel, ElfW(Sym) *sym,
+		    const char *symname)
 {
 	unsigned int r_type = ELF64_R_TYPE(rel->r_info);
 	ElfW(Addr) offset = rel->r_offset;
@@ -296,21 +422,29 @@ static void walk_relocs(void)
 
 	/* Walk through the relocations */
 	for (i = 0; i < shnum; i++) {
-		struct section *sec_applies;
+		char *sym_strtab;
+		Elf_Sym *sh_symtab;
+		struct section *sec_applies, *sec_symtab;
 		int j;
 		struct section *sec = &secs[i];
 
 		if (sec->shdr.sh_type != SHT_REL_TYPE)
 			continue;
 
+		sec_symtab  = sec->link;
 		sec_applies = &secs[sec->shdr.sh_info];
 		if (!(sec_applies->shdr.sh_flags & SHF_ALLOC))
 			continue;
 
+		sh_symtab = sec_symtab->symtab;
+		sym_strtab = sec_symtab->link->strtab;
+
 		for (j = 0; j < sec->shdr.sh_size / sizeof(Elf_Rel); j++) {
 			Elf_Rel *rel = &sec->reltab[j];
+			Elf_Sym *sym = &sh_symtab[ELF_R_SYM(rel->r_info)];
+			const char *symname = sym_name(sym_strtab, sym);
 
-			do_reloc(sec, rel);
+			do_reloc(sec, rel, sym, symname);
 		}
 	}
 }
@@ -349,6 +483,8 @@ static void process(FILE *fp)
 {
 	read_ehdr(fp);
 	read_shdrs(fp);
+	read_strtabs(fp);
+	read_symtabs(fp);
 	read_relocs(fp);
 	emit_relocs();
 }
-- 
2.53.0

[PATCH 2/6] s390/tools/relocs: Ignore __kcfi_typeid_ relocations

[Heiko Carstens]

This is the s390 variant of commit ca7e10bff196 ("x86/tools/relocs: Ignore
__kcfi_typeid_ relocations"):

"The compiler generates __kcfi_typeid_ symbols for annotating assembly
functions with type information. These are constants that can be referenced
in assembly code and are resolved by the linker. Ignore them in relocs."

Reviewed-by: Jens Remus <jremus@linux.ibm.com>
Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
---
 arch/s390/tools/relocs.c | 16 ++++++++++++++++
 1 file changed, 16 insertions(+)

diff --git a/arch/s390/tools/relocs.c b/arch/s390/tools/relocs.c
index a2774176d38f..72178dc90a4c 100644
--- a/arch/s390/tools/relocs.c
+++ b/arch/s390/tools/relocs.c
@@ -405,6 +405,22 @@ static int do_reloc(struct section *sec, Elf_Rel *rel, ElfW(Sym) *sym,
 	case R_390_GOTPCDBL:
 	case R_390_GOTOFF64:
 		break;
+	case R_390_32: {
+		static const char kcfipfx[] = "__kcfi_typeid_";
+
+		if (sym->st_shndx != SHN_ABS)
+			die("Unsupported relocation type: %d\n", r_type);
+		/*
+		 * Symbols with __kcfi_typeid_ prefix have constant values,
+		 * which do not change if bzImage is loaded at a different
+		 * physical address than the address for which it has been
+		 * compiled.
+		 */
+		if (!strncmp(kcfipfx, symname, sizeof(kcfipfx) - 1))
+			break;
+		die("Invalid absolute R_390_32 relocation: %s\n", symname);
+		break;
+	}
 	case R_390_64:
 		add_reloc(&relocs64, offset);
 		break;
-- 
2.53.0

[PATCH 3/6] s390: Add ftrace_stub_graph

[Heiko Carstens]

This is the s390 variant of commit f3a0c23f2539 ("riscv: Add
ftrace_stub_graph"):

"Commit 883bbbffa5a4 ("ftrace,kcfi: Separate ftrace_stub() and
ftrace_stub_graph()") added a separate ftrace_stub_graph function for
CFI_CLANG. Add the stub to fix FUNCTION_GRAPH_TRACER compatibility
with CFI."

Reviewed-by: Jens Remus <jremus@linux.ibm.com>
Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
---
 arch/s390/kernel/mcount.S | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/arch/s390/kernel/mcount.S b/arch/s390/kernel/mcount.S
index 1fec370fecf4..97e9d6bdc017 100644
--- a/arch/s390/kernel/mcount.S
+++ b/arch/s390/kernel/mcount.S
@@ -4,6 +4,7 @@
  *
  */
 
+#include <linux/cfi_types.h>
 #include <linux/linkage.h>
 #include <asm/asm-offsets.h>
 #include <asm/ftrace.h>
@@ -34,10 +35,14 @@
 
 	.section .kprobes.text, "ax"
 
-SYM_FUNC_START(ftrace_stub)
+SYM_TYPED_FUNC_START(ftrace_stub)
 	BR_EX	%r14
 SYM_FUNC_END(ftrace_stub)
 
+SYM_TYPED_FUNC_START(ftrace_stub_graph)
+	BR_EX	%r14
+SYM_FUNC_END(ftrace_stub_graph)
+
 SYM_CODE_START(ftrace_stub_direct_tramp)
 	lgr	%r1, %r0
 	BR_EX	%r1
-- 
2.53.0

[PATCH 4/6] s390/diag: Generate CFI type information for assembly functions

[Heiko Carstens]

Use SYM_TYPED_FUNC_START to generate __kcfi_typeid_ symbols for assembler
functions which are called indirectly. All assembler functions contained in
text_amode31.S are called indirectly and require such annotations.

Reviewed-by: Jens Remus <jremus@linux.ibm.com>
Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
---
 arch/s390/kernel/text_amode31.S | 13 +++++++------
 1 file changed, 7 insertions(+), 6 deletions(-)

diff --git a/arch/s390/kernel/text_amode31.S b/arch/s390/kernel/text_amode31.S
index 26f2981aa09e..f007d892d0c5 100644
--- a/arch/s390/kernel/text_amode31.S
+++ b/arch/s390/kernel/text_amode31.S
@@ -5,6 +5,7 @@
  * Copyright IBM Corp. 2019
  */
 
+#include <linux/cfi_types.h>
 #include <linux/linkage.h>
 #include <asm/asm-extable.h>
 #include <asm/errno.h>
@@ -26,7 +27,7 @@
 /*
  * int _diag14_amode31(unsigned long rx, unsigned long ry1, unsigned long subcode)
  */
-SYM_FUNC_START(_diag14_amode31)
+SYM_TYPED_FUNC_START(_diag14_amode31)
 	lgr	%r1,%r2
 	lgr	%r2,%r3
 	lgr	%r3,%r4
@@ -46,7 +47,7 @@ SYM_FUNC_END(_diag14_amode31)
 /*
  * int _diag210_amode31(struct diag210 *addr)
  */
-SYM_FUNC_START(_diag210_amode31)
+SYM_TYPED_FUNC_START(_diag210_amode31)
 	lgr	%r1,%r2
 	lhi	%r2,-1
 	sam31
@@ -64,7 +65,7 @@ SYM_FUNC_END(_diag210_amode31)
 /*
  * int diag8c(struct diag8c *addr, struct ccw_dev_id *devno, size_t len)
 */
-SYM_FUNC_START(_diag8c_amode31)
+SYM_TYPED_FUNC_START(_diag8c_amode31)
 	llgf	%r3,0(%r3)
 	sam31
 	diag	%r2,%r4,0x8c
@@ -77,7 +78,7 @@ SYM_FUNC_END(_diag8c_amode31)
 /*
  * int _diag26c_amode31(void *req, void *resp, enum diag26c_sc subcode)
  */
-SYM_FUNC_START(_diag26c_amode31)
+SYM_TYPED_FUNC_START(_diag26c_amode31)
 	lghi	%r5,-EOPNOTSUPP
 	sam31
 	diag	%r2,%r4,0x26c
@@ -91,7 +92,7 @@ SYM_FUNC_END(_diag26c_amode31)
 /*
  * void _diag0c_amode31(unsigned long rx)
  */
-SYM_FUNC_START(_diag0c_amode31)
+SYM_TYPED_FUNC_START(_diag0c_amode31)
 	sam31
 	diag	%r2,%r2,0x0c
 	sam64
@@ -103,7 +104,7 @@ SYM_FUNC_END(_diag0c_amode31)
  *
  * Calls diag 308 subcode 1 and continues execution
  */
-SYM_FUNC_START(_diag308_reset_amode31)
+SYM_TYPED_FUNC_START(_diag308_reset_amode31)
 	larl	%r4,ctlregs		# Save control registers
 	stctg	%c0,%c15,0(%r4)
 	lg	%r2,0(%r4)		# Disable lowcore protection
-- 
2.53.0

[PATCH 5/6] s390/Kconfig: Select ARCH_SUPPORTS_CFI

[Heiko Carstens]

With all prerequisites in place select ARCH_SUPPORTS_CFI.

Note that this support is supposed to work with the generic kcfi support
which is provided by clang. This comes with a couple of limitations:

The generic kcfi implementation does not generate a .kcfi_traps section,
nor is a special instruction used in case a checksum mismatch is detected.

This means in case of checksum mismatch the kernel just crashes. It should
be quite easy to tell by the surrounding code that a crash happened because
of a checksum mismatch.

If clang and/or gcc provide a .kcfi_traps section it will be possible to
print proper CFI messages instead of just crashing the kernel (enable
ARCH_USES_CFI_TRAPS).

In addition this also means that CFI_PERMISSIVE does not work. Even if the
option is selected the kernel will crash in case of checksum mismatch.

However it seems to be acceptable to enable kcfi support to the kernel now
even if it is not perfect. Later clang and gcc extensions are required to
improve this.

As of now a crash caused by a CFI failure looks like this:

illegal operation: 0001 ilc:1 [#1]SMP
Modules linked in: bpf_testmod(OE)
CPU: 0 UID: 0 PID: 92 Comm: test_progs Tainted: G           OE       7.2.0-rc4-00021-gc35ed7a1ca22-dirty #3 PREEMPTLAZY
Tainted: [O]=OOT_MODULE, [E]=UNSIGNED_MODULE
Hardware name: IBM 3931 A01 703 (KVM/Linux)
Krnl PSW : 0704e00180000000 00000166d4853a0a (bpf_task_work_callback+0x176/0x290)
           R:0 T:1 IO:1 EX:1 Key:0 M:1 W:0 P:0 AS:3 CC:2 PM:0 RI:0 EA:3
Krnl GPRS: 0400000069b02e96 000001665471856c 0000000084dc1000 000000008084da58
           000000008084da60 000000005ff492bf 0000000000000000 00000000809de300
           fffffffffff7ffff 00000000000a0337 00000000809e4d00 000000008437b100
           00000000801bc288 00000000801bc280 00000166d48538fc 000000e6d502ba90
Krnl Code: 00000166d48539fa: e320c0400004       lg      %r2,64(%r12)
           00000166d4853a00: e340c0480004       lg      %r4,72(%r12)
          *00000166d4853a06: a7640001           brc     6,00000166d4853a08
          >00000166d4853a0a: 0de1               basr    %r14,%r1
           00000166d4853a0c: e300b6580095       llh     %r0,1624(%r11)
           00000166d4853a12: ec040009027e       cij     %r0,2,4,00000166d4853a24
           00000166d4853a18: a70affff           ahi     %r0,-1
           00000166d4853a1c: 4000b658           sth     %r0,1624(%r11)
Call Trace:
 [<00000166d4853a0a>] bpf_task_work_callback+0x176/0x290
...

The disassembly contains the brc instruction (branch into the instruction
itself with two byte offset to execute an illegal instruction, and the
following basr instruction used for an indirect branch.

Reviewed-by: Jens Remus <jremus@linux.ibm.com>
Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
---
 arch/s390/Kconfig | 1 +
 1 file changed, 1 insertion(+)

diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
index 84404e6778d5..d57a6c02f51d 100644
--- a/arch/s390/Kconfig
+++ b/arch/s390/Kconfig
@@ -144,6 +144,7 @@ config S390
 	select ARCH_MHP_MEMMAP_ON_MEMORY_ENABLE
 	select ARCH_STACKWALK
 	select ARCH_SUPPORTS_ATOMIC_RMW
+	select ARCH_SUPPORTS_CFI
 	select ARCH_SUPPORTS_DEBUG_PAGEALLOC
 	select ARCH_SUPPORTS_HUGETLBFS
 	select ARCH_SUPPORTS_INT128 if CC_HAS_INT128 && CC_IS_CLANG
-- 
2.53.0

[PATCH 6/6] s390/bpf: Add kCFI support

[Heiko Carstens]

This is the s390 variant of commit 710618c760c0 ("arm64/cfi,bpf: Support
kCFI + BPF on arm64").

Signed-off-by: Heiko Carstens <hca@linux.ibm.com>
---
 arch/s390/include/asm/cfi.h  |  7 +++++++
 arch/s390/net/bpf_jit_comp.c | 26 ++++++++++++++++++++++----
 2 files changed, 29 insertions(+), 4 deletions(-)
 create mode 100644 arch/s390/include/asm/cfi.h

diff --git a/arch/s390/include/asm/cfi.h b/arch/s390/include/asm/cfi.h
new file mode 100644
index 000000000000..9af2c7cb70ca
--- /dev/null
+++ b/arch/s390/include/asm/cfi.h
@@ -0,0 +1,7 @@
+/* SPDX-License-Identifier: GPL-2.0 */
+#ifndef _ASM_S390_CFI_H
+#define _ASM_S390_CFI_H
+
+#define __bpfcall
+
+#endif /* _ASM_S390_CFI_H */
diff --git a/arch/s390/net/bpf_jit_comp.c b/arch/s390/net/bpf_jit_comp.c
index 31749c0362ca..74642e0b4ee1 100644
--- a/arch/s390/net/bpf_jit_comp.c
+++ b/arch/s390/net/bpf_jit_comp.c
@@ -21,6 +21,7 @@
 #include <linux/filter.h>
 #include <linux/init.h>
 #include <linux/bpf.h>
+#include <linux/cfi.h>
 #include <linux/mm.h>
 #include <linux/kernel.h>
 #include <asm/cacheflush.h>
@@ -356,6 +357,19 @@ static void emit6_pcrel_rilc(struct bpf_jit *jit, u32 op, u8 mask, s64 pcrel)
 	}							\
 })
 
+static inline void emit_u32_data(const u32 data, struct bpf_jit *jit)
+{
+	if (jit->prg_buf)
+		*(u32 *)(jit->prg_buf + jit->prg) = data;
+	jit->prg += 4;
+}
+
+static inline void emit_kcfi(u32 hash, struct bpf_jit *jit)
+{
+	if (IS_ENABLED(CONFIG_CFI))
+		emit_u32_data(hash, jit);
+}
+
 /*
  * Return whether this is the first pass. The first pass is special, since we
  * don't know any sizes yet, and thus must be conservative.
@@ -597,6 +611,8 @@ static void bpf_jit_prologue(struct bpf_jit *jit, struct bpf_prog *fp)
 {
 	BUILD_BUG_ON(sizeof(struct prog_frame) != STACK_FRAME_OVERHEAD);
 
+	emit_kcfi(bpf_is_subprog(fp) ? cfi_bpf_subprog_hash : cfi_bpf_hash, jit);
+
 	/* No-op for hotpatching */
 	/* brcl 0,prologue_plt */
 	EMIT6_PCREL_RILC(0xc0040000, 0, jit->prologue_plt);
@@ -616,7 +632,7 @@ static void bpf_jit_prologue(struct bpf_jit *jit, struct bpf_prog *fp)
 		bpf_skip(jit, 6);
 	}
 	/* Tail calls have to skip above initialization */
-	jit->tail_call_start = jit->prg;
+	jit->tail_call_start = jit->prg - cfi_get_offset();
 	if (fp->aux->exception_cb) {
 		/*
 		 * Switch stack, the new address is in the 2nd parameter.
@@ -2401,9 +2417,9 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_verifier_env *env, struct bpf_pr
 		jit_data->ctx = jit;
 		jit_data->pass = pass;
 	}
-	fp->bpf_func = (void *) jit.prg_buf;
+	fp->bpf_func = (void *)jit.prg_buf + cfi_get_offset();
 	fp->jited = 1;
-	fp->jited_len = jit.size;
+	fp->jited_len = jit.size - cfi_get_offset();
 
 	if (!fp->is_func || extra_pass) {
 		bpf_prog_fill_jited_linfo(fp, jit.addrs + 1);
@@ -2671,8 +2687,10 @@ static int __arch_prepare_bpf_trampoline(struct bpf_tramp_image *im,
 		return -ENOTSUPP;
 
 	/* Return to %r14 in the struct_ops case. */
-	if (flags & BPF_TRAMP_F_INDIRECT)
+	if (flags & BPF_TRAMP_F_INDIRECT) {
 		flags |= BPF_TRAMP_F_SKIP_FRAME;
+		emit_kcfi(cfi_get_func_hash(func_addr), jit);
+	}
 
 	/*
 	 * Compute how many arguments we need to pass to BPF programs.
-- 
2.53.0

Re: [PATCH 3/6] s390: Add ftrace_stub_graph

[sashiko-bot]

> This is the s390 variant of commit f3a0c23f2539 ("riscv: Add
> ftrace_stub_graph"):
> 
> "Commit 883bbbffa5a4 ("ftrace,kcfi: Separate ftrace_stub() and
> ftrace_stub_graph()") added a separate ftrace_stub_graph function for
> CFI_CLANG. Add the stub to fix FUNCTION_GRAPH_TRACER compatibility
> with CFI."
> 
> Reviewed-by: Jens Remus <jremus@linux.ibm.com>
> Signed-off-by: Heiko Carstens <hca@linux.ibm.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260724141318.1037434-1-hca@linux.ibm.com?part=3

Re: [PATCH 1/6] s390/tools: Pass symbol name to do_relocs()

[sashiko-bot]

> Pass the symbol symbol name which corresponds to a relocation to
> do_relocs(). This is preparation for kcfi support.
> 
> Given that the s390 specific relocs tool is a stripped down version of the
> x86 version, add more code from the x86 version to the s390 version, while
> keeping coding style, etc. in order to add the required functionality.
> 
> Reviewed-by: Jens Remus <jremus@linux.ibm.com>
> Signed-off-by: Heiko Carstens <hca@linux.ibm.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260724141318.1037434-1-hca@linux.ibm.com?part=1

Re: [PATCH 4/6] s390/diag: Generate CFI type information for assembly functions

[sashiko-bot]

> Use SYM_TYPED_FUNC_START to generate __kcfi_typeid_ symbols for assembler
> functions which are called indirectly. All assembler functions contained in
> text_amode31.S are called indirectly and require such annotations.
> 
> Reviewed-by: Jens Remus <jremus@linux.ibm.com>
> Signed-off-by: Heiko Carstens <hca@linux.ibm.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260724141318.1037434-1-hca@linux.ibm.com?part=4

Re: [PATCH 2/6] s390/tools/relocs: Ignore __kcfi_typeid_ relocations

[sashiko-bot]

> This is the s390 variant of commit ca7e10bff196 ("x86/tools/relocs: Ignore
> __kcfi_typeid_ relocations"):
> 
> "The compiler generates __kcfi_typeid_ symbols for annotating assembly
> functions with type information. These are constants that can be referenced
> in assembly code and are resolved by the linker. Ignore them in relocs."
> 
> Reviewed-by: Jens Remus <jremus@linux.ibm.com>
> Signed-off-by: Heiko Carstens <hca@linux.ibm.com>

Sashiko has reviewed this patch and found no issues. It looks great!

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260724141318.1037434-1-hca@linux.ibm.com?part=2

Re: [PATCH 6/6] s390/bpf: Add kCFI support

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:
- [Medium] BPF line info offsets are shifted by 4 bytes due to double-counting the CFI hash size in `bpf_prog_fill_jited_linfo()`.
--

--- Patch [6]: [PATCH 6/6] s390/bpf: Add kCFI support ---
commit 07d29b72daf2643a639d32a0ab6a9b913adcfbc6
Author: Heiko Carstens <hca@linux.ibm.com>

s390/bpf: Add kCFI support

This patch adds kCFI (Kernel Control Flow Integrity) support for BPF programs 
on the s390 architecture. It emits a CFI hash before the BPF program 
instructions and adjusts the function pointer and lengths to account for 
this prefix.

> diff --git a/arch/s390/net/bpf_jit_comp.c b/arch/s390/net/bpf_jit_comp.c
> index 31749c0362ca2..74642e0b4ee19 100644
> --- a/arch/s390/net/bpf_jit_comp.c
> +++ b/arch/s390/net/bpf_jit_comp.c

[ ... ]

> @@ -2401,9 +2417,9 @@ struct bpf_prog *bpf_int_jit_compile(struct bpf_verifier_env *env, struct bpf_pr
>  		jit_data->ctx = jit;
>  		jit_data->pass = pass;
>  	}
> -	fp->bpf_func = (void *) jit.prg_buf;
> +	fp->bpf_func = (void *)jit.prg_buf + cfi_get_offset();
>  	fp->jited = 1;
> -	fp->jited_len = jit.size;
> +	fp->jited_len = jit.size - cfi_get_offset();
>  
>  	if (!fp->is_func || extra_pass) {
>  		bpf_prog_fill_jited_linfo(fp, jit.addrs + 1);

[Severity: Medium]
Does this double-count the CFI offset when calculating the JIT line info?

Looking at bpf_int_jit_compile(), the emit_kcfi() call during the prologue 
generation advances jit.prg by 4 bytes. This means the offsets stored in 
jit.addrs already include the CFI hash size.

However, when we pass these offsets to bpf_prog_fill_jited_linfo(), it 
uses fp->bpf_func as the base address to resolve the final instruction 
pointers:

kernel/bpf/core.c:bpf_prog_fill_jited_linfo() {
    ...
    for (i = 1; i < nr_linfo && linfo[i].insn_off < insn_end; i++)
        jited_linfo[i] = prog->bpf_func +
            insn_to_jit_off[linfo[i].insn_off - insn_start - 1];
}

Since fp->bpf_func has already been shifted forward by cfi_get_offset(), 
adding the unadjusted jit.addrs offsets might shift the resulting line info 
by 4 bytes, which could break source line mapping in userspace tools.

Should jit.addrs be adjusted to subtract cfi_get_offset() before filling 
the line info, or should the original base pointer be passed instead?

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260724141318.1037434-1-hca@linux.ibm.com?part=6

Re: [PATCH 5/6] s390/Kconfig: Select ARCH_SUPPORTS_CFI

[sashiko-bot]

Thank you for your contribution! Sashiko AI review found 2 potential issue(s) to consider:
- [High] Missing CFI type hashes in s390 BPF JIT causes kernel panics on valid BPF callbacks.
- [Medium] Enabling ARCH_SUPPORTS_CFI on s390 breaks the semantic contract of CFI_PERMISSIVE, causing kernel panics instead of warnings.
--

--- Patch [5]: [PATCH 5/6] s390/Kconfig: Select ARCH_SUPPORTS_CFI ---
commit 331ca22524bf97cd2604c69fc257401120f48c71
Author: Heiko Carstens <hca@linux.ibm.com>

s390/Kconfig: Select ARCH_SUPPORTS_CFI

This commit selects ARCH_SUPPORTS_CFI for s390 to enable KCFI support. It
notes that without a .kcfi_traps section, CFI mismatches will result in
kernel crashes instead of warnings, even in permissive mode.

> In addition this also means that CFI_PERMISSIVE does not work. Even if the
> option is selected the kernel will crash in case of checksum mismatch.

[Severity: Medium]
Since enabling this feature on s390 breaks the expected semantic contract
of CFI_PERMISSIVE defined in arch/Kconfig, should we actively prevent users
from selecting permissive mode on s390?

Currently, a user configuring CONFIG_CFI and CONFIG_CFI_PERMISSIVE would
expect a warning on a mismatch, but will experience a hard panic instead.

Would it be better to add a dependency restricting CFI_PERMISSIVE so it
cannot be selected when the architecture lacks trap interception?

> diff --git a/arch/s390/Kconfig b/arch/s390/Kconfig
> index 84404e6778d50..d57a6c02f51d2 100644
> --- a/arch/s390/Kconfig
> +++ b/arch/s390/Kconfig
> @@ -144,6 +144,7 @@ config S390
>  	select ARCH_MHP_MEMMAP_ON_MEMORY_ENABLE
>  	select ARCH_STACKWALK
>  	select ARCH_SUPPORTS_ATOMIC_RMW
> +	select ARCH_SUPPORTS_CFI

[Severity: High]
Does the s390 BPF JIT implementation currently generate the required CFI type
hashes?

Looking at bpf_int_jit_compile() in arch/s390/net/bpf_jit_comp.c, if the
JIT does not yet emit the cfi_bpf_hash preceding the generated code, will
valid BPF subprogram callbacks (like bpf_task_work_callback or timers)
trigger a KCFI mismatch and crash the host?

>  	select ARCH_SUPPORTS_DEBUG_PAGEALLOC
>  	select ARCH_SUPPORTS_HUGETLBFS
>  	select ARCH_SUPPORTS_INT128 if CC_HAS_INT128 && CC_IS_CLANG

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260724141318.1037434-1-hca@linux.ibm.com?part=5

Re: [PATCH 1/6] s390/tools: Pass symbol name to do_relocs()

[bot+bpf-ci]

[-- Attachment #1: Type: text/plain, Size: 734 bytes --]

> Pass the symbol symbol name which corresponds to a relocation to
> do_relocs(). This is preparation for kcfi support.
>
> Given that the s390 specific relocs tool is a stripped down version of the
> x86 version, add more code from the x86 version to the s390 version, while
> keeping coding style, etc. in order to add the required functionality.

This isn't a bug, but "the symbol symbol name" in the first sentence looks
like it has "symbol" repeated.  Should that read "the symbol name"?


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/30100858043

Re: [PATCH 3/6] s390: Add ftrace_stub_graph

[bot+bpf-ci]

[-- Attachment #1: Type: text/plain, Size: 2088 bytes --]

> diff --git a/arch/s390/kernel/mcount.S b/arch/s390/kernel/mcount.S
> index 1fec370fecf48..97e9d6bdc0174 100644
> --- a/arch/s390/kernel/mcount.S
> +++ b/arch/s390/kernel/mcount.S
> @@ -4,6 +4,7 @@
>   *
>   */
>
> +#include <linux/cfi_types.h>
>  #include <linux/linkage.h>
>  #include <asm/asm-offsets.h>
>  #include <asm/ftrace.h>

[ ... ]

> @@ -34,10 +35,14 @@
>
>  	.section .kprobes.text, "ax"
>
> -SYM_FUNC_START(ftrace_stub)
> +SYM_TYPED_FUNC_START(ftrace_stub)
>  	BR_EX	%r14
>  SYM_FUNC_END(ftrace_stub)
>
> +SYM_TYPED_FUNC_START(ftrace_stub_graph)
> +	BR_EX	%r14
> +SYM_FUNC_END(ftrace_stub_graph)

Will this link when CONFIG_CFI_CLANG=y and CONFIG_FUNCTION_GRAPH_TRACER=n?

When CFI is enabled, the SYM_TYPED_FUNC_START() macro emits a reference to
__kcfi_typeid_ftrace_stub_graph. However, this symbol is only generated if
ftrace_stub_graph is declared in C code, which only happens when
CONFIG_FUNCTION_GRAPH_TRACER is enabled in kernel/trace/fgraph.c.

Both the RISC-V reference implementation (commit f3a0c23f2539) and the x86
fix (commit aa69f814920d8) wrap ftrace_stub_graph inside:

  #ifdef CONFIG_FUNCTION_GRAPH_TRACER
  SYM_TYPED_FUNC_START(ftrace_stub_graph)
  	BR_EX	%r14
  SYM_FUNC_END(ftrace_stub_graph)
  #endif

From commit aa69f814920d8:

  "When CONFIG_FUNCTION_GRAPH_TRACER is disabled,
  __kcfi_typeid_ftrace_stub_graph is missing, causing a link failure:
  ld.lld: error: undefined symbol: __kcfi_typeid_ftrace_stub_graph
  referenced by arch/x86/kernel/ftrace_64.o:(__cfi_ftrace_stub_graph) in
  archive vmlinux.a"

Should the s390 implementation include the CONFIG_FUNCTION_GRAPH_TRACER
guard?

> +
>  SYM_CODE_START(ftrace_stub_direct_tramp)
>  	lgr	%r1, %r0
>  	BR_EX	%r1

This looks like a bug fix. Should this include:

  Fixes: 883bbbffa5a4 ("ftrace,kcfi: Separate ftrace_stub() and ftrace_stub_graph()")


---
AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md

CI run summary: https://github.com/kernel-patches/bpf/actions/runs/30100858043