Adds CFI checks to BPF dispatchers on aarch64.
E.g.
<bpf_dispatcher_*_func>:
paciasp
stp x29, x30, [sp, #-0x10]!
mov x29, sp
+ ldur w16, [x2, #-0x4]
+ movk w17, #0x1881
+ movk w17, #0xd942, lsl #16
+ cmp w16, w17
+ b.eq <bpf_dispatcher_*_func+0x24>
+ brk #0x8222
blr x2
ldp x29, x30, [sp], #0x10
autiasp
ret
Changes in v4->v5
https://lore.kernel.org/all/wtb6czzpvtqq23t4g6hf7on257dtxzdb4fa4nuq3dtq32odmli@xoyyrtthafar/
- Fix failing BPF selftests from misplaced variable declaration
Changes in v3->v4
https://lore.kernel.org/all/fhdcjdzqdqnoehenxbipfaorseeamt3q7fbm7ghe6z5s2chif5@lrhtasolawud/
- Fix authorship attribution.
Changes in v2->v3:
https://lore.kernel.org/all/[email protected]/
- Simplify cfi_get_func_hash to avoid needless failure case
- Use DEFINE_CFI_TYPE as suggested by Mark Rutland
Changes in v1->v2:
https://lore.kernel.org/bpf/[email protected]/
- Rebased on latest bpf-next/master
Mark Rutland (1):
cfi: add C CFI type macro
Maxwell Bland (1):
arm64/cfi,bpf: Use DEFINE_CFI_TYPE in arm64
Puranjay Mohan (1):
arm64/cfi,bpf: Support kCFI + BPF on arm64
arch/arm64/include/asm/cfi.h | 23 ++++++++++++++++++++++
arch/arm64/kernel/alternative.c | 18 +++++++++++++++++
arch/arm64/net/bpf_jit_comp.c | 21 +++++++++++++++++---
arch/riscv/kernel/cfi.c | 34 ++------------------------------
arch/x86/kernel/alternative.c | 35 +++------------------------------
include/linux/cfi_types.h | 23 ++++++++++++++++++++++
6 files changed, 87 insertions(+), 67 deletions(-)
create mode 100644 arch/arm64/include/asm/cfi.h
--
Sorry for the extreme delay Puranjay and other maintainers on the
submission for this. The past month I was on incident response rotation
here at Moto and my hands were full with scripting build scanning steps
and other product deployment nonsense. Better late than never, though,
if these changes have not been merged yet. (-:
Tested on a cortex-a76 qemu instance and self-tests are matching the
baseline bpf-next success rate (Summary: 509/3700 PASSED, 77 SKIPPED, 37
FAILED).
Thanks for your review and regards,
Maxwell
2.39.2
From: Mark Rutland <[email protected]>
Currently x86 and riscv open-code 4 instances of the same logic to
define a u32 variable with the KCFI typeid of a given function.
Replace the duplicate logic with a common macro.
Signed-off-by: Mark Rutland <[email protected]>
---
arch/riscv/kernel/cfi.c | 34 ++--------------------------------
arch/x86/kernel/alternative.c | 35 +++--------------------------------
include/linux/cfi_types.h | 23 +++++++++++++++++++++++
3 files changed, 28 insertions(+), 64 deletions(-)
diff --git a/arch/riscv/kernel/cfi.c b/arch/riscv/kernel/cfi.c
index 64bdd3e1ab8c..b78a6f41df22 100644
--- a/arch/riscv/kernel/cfi.c
+++ b/arch/riscv/kernel/cfi.c
@@ -82,41 +82,11 @@ struct bpf_insn;
/* Must match bpf_func_t / DEFINE_BPF_PROG_RUN() */
extern unsigned int __bpf_prog_runX(const void *ctx,
const struct bpf_insn *insn);
-
-/*
- * Force a reference to the external symbol so the compiler generates
- * __kcfi_typid.
- */
-__ADDRESSABLE(__bpf_prog_runX);
-
-/* u32 __ro_after_init cfi_bpf_hash = __kcfi_typeid___bpf_prog_runX; */
-asm (
-" .pushsection .data..ro_after_init,\"aw\",@progbits \n"
-" .type cfi_bpf_hash,@object \n"
-" .globl cfi_bpf_hash \n"
-" .p2align 2, 0x0 \n"
-"cfi_bpf_hash: \n"
-" .word __kcfi_typeid___bpf_prog_runX \n"
-" .size cfi_bpf_hash, 4 \n"
-" .popsection \n"
-);
+DEFINE_CFI_TYPE(cfi_bpf_hash, __bpf_prog_runX);
/* Must match bpf_callback_t */
extern u64 __bpf_callback_fn(u64, u64, u64, u64, u64);
-
-__ADDRESSABLE(__bpf_callback_fn);
-
-/* u32 __ro_after_init cfi_bpf_subprog_hash = __kcfi_typeid___bpf_callback_fn; */
-asm (
-" .pushsection .data..ro_after_init,\"aw\",@progbits \n"
-" .type cfi_bpf_subprog_hash,@object \n"
-" .globl cfi_bpf_subprog_hash \n"
-" .p2align 2, 0x0 \n"
-"cfi_bpf_subprog_hash: \n"
-" .word __kcfi_typeid___bpf_callback_fn \n"
-" .size cfi_bpf_subprog_hash, 4 \n"
-" .popsection \n"
-);
+DEFINE_CFI_TYPE(cfi_bpf_subprog_hash, __bpf_callback_fn);
u32 cfi_get_func_hash(void *func)
{
diff --git a/arch/x86/kernel/alternative.c b/arch/x86/kernel/alternative.c
index 89de61243272..933d0c13a0d8 100644
--- a/arch/x86/kernel/alternative.c
+++ b/arch/x86/kernel/alternative.c
@@ -1,6 +1,7 @@
// SPDX-License-Identifier: GPL-2.0-only
#define pr_fmt(fmt) "SMP alternatives: " fmt
+#include <linux/cfi_types.h>
#include <linux/module.h>
#include <linux/sched.h>
#include <linux/perf_event.h>
@@ -901,41 +902,11 @@ struct bpf_insn;
/* Must match bpf_func_t / DEFINE_BPF_PROG_RUN() */
extern unsigned int __bpf_prog_runX(const void *ctx,
const struct bpf_insn *insn);
-
-/*
- * Force a reference to the external symbol so the compiler generates
- * __kcfi_typid.
- */
-__ADDRESSABLE(__bpf_prog_runX);
-
-/* u32 __ro_after_init cfi_bpf_hash = __kcfi_typeid___bpf_prog_runX; */
-asm (
-" .pushsection .data..ro_after_init,\"aw\",@progbits \n"
-" .type cfi_bpf_hash,@object \n"
-" .globl cfi_bpf_hash \n"
-" .p2align 2, 0x0 \n"
-"cfi_bpf_hash: \n"
-" .long __kcfi_typeid___bpf_prog_runX \n"
-" .size cfi_bpf_hash, 4 \n"
-" .popsection \n"
-);
+DEFINE_CFI_TYPE(cfi_bpf_hash, __bpf_prog_runX);
/* Must match bpf_callback_t */
extern u64 __bpf_callback_fn(u64, u64, u64, u64, u64);
-
-__ADDRESSABLE(__bpf_callback_fn);
-
-/* u32 __ro_after_init cfi_bpf_subprog_hash = __kcfi_typeid___bpf_callback_fn; */
-asm (
-" .pushsection .data..ro_after_init,\"aw\",@progbits \n"
-" .type cfi_bpf_subprog_hash,@object \n"
-" .globl cfi_bpf_subprog_hash \n"
-" .p2align 2, 0x0 \n"
-"cfi_bpf_subprog_hash: \n"
-" .long __kcfi_typeid___bpf_callback_fn \n"
-" .size cfi_bpf_subprog_hash, 4 \n"
-" .popsection \n"
-);
+DEFINE_CFI_TYPE(cfi_bpf_subprog_hash, __bpf_callback_fn);
u32 cfi_get_func_hash(void *func)
{
diff --git a/include/linux/cfi_types.h b/include/linux/cfi_types.h
index 6b8713675765..f510e62ca8b1 100644
--- a/include/linux/cfi_types.h
+++ b/include/linux/cfi_types.h
@@ -41,5 +41,28 @@
SYM_TYPED_START(name, SYM_L_GLOBAL, SYM_A_ALIGN)
#endif
+#else /* __ASSEMBLY__ */
+
+#ifdef CONFIG_CFI_CLANG
+#define DEFINE_CFI_TYPE(name, func) \
+ /* \
+ * Force a reference to the function so the compiler generates \
+ * __kcfi_typeid_<func>. \
+ */ \
+ __ADDRESSABLE(func); \
+ /* u32 name = __kcfi_typeid_<func> */ \
+ extern u32 name; \
+ asm ( \
+ " .pushsection .data..ro_after_init,\"aw\",@progbits \n" \
+ " .type " #name ",@object \n" \
+ " .globl " #name " \n" \
+ " .p2align 2, 0x0 \n" \
+ #name ": \n" \
+ " .long __kcfi_typeid_" #func " \n" \
+ " .size " #name ", 4 \n" \
+ " .popsection \n" \
+ );
+#endif
+
#endif /* __ASSEMBLY__ */
#endif /* _LINUX_CFI_TYPES_H */
--
2.39.2
Hi Maxwell,
kernel test robot noticed the following build errors:
[auto build test ERROR on bpf-next/master]
url: https://github.com/intel-lab-lkp/linux/commits/Maxwell-Bland/arm64-cfi-bpf-Support-kCFI-BPF-on-arm64/20240611-021203
base: https://git.kernel.org/pub/scm/linux/kernel/git/bpf/bpf-next.git master
patch link: https://lore.kernel.org/r/cwhnmpn5yvg6ma7mvjviy4p7z6gdoba57daeprpc4zcokfhpv2%4044gvdmcfuspt
patch subject: [PATCH bpf-next v5 1/3] cfi: add C CFI type macro
config: riscv-allmodconfig (https://download.01.org/0day-ci/archive/20240611/[email protected]/config)
compiler: clang version 19.0.0git (https://github.com/llvm/llvm-project 4403cdbaf01379de96f8d0d6ea4f51a085e37766)
reproduce (this is a W=1 build): (https://download.01.org/0day-ci/archive/20240611/[email protected]/reproduce)
If you fix the issue in a separate patch/commit (i.e. not just a new version of
the same patch/commit), kindly add following tags
| Reported-by: kernel test robot <[email protected]>
| Closes: https://lore.kernel.org/oe-kbuild-all/[email protected]/
All errors (new ones prefixed by >>):
>> arch/riscv/kernel/cfi.c:85:1: error: type specifier missing, defaults to 'int'; ISO C99 and later do not support implicit int [-Wimplicit-int]
85 | DEFINE_CFI_TYPE(cfi_bpf_hash, __bpf_prog_runX);
| ^
| int
>> arch/riscv/kernel/cfi.c:85:17: error: a parameter list without types is only allowed in a function definition
85 | DEFINE_CFI_TYPE(cfi_bpf_hash, __bpf_prog_runX);
| ^
arch/riscv/kernel/cfi.c:89:1: error: type specifier missing, defaults to 'int'; ISO C99 and later do not support implicit int [-Wimplicit-int]
89 | DEFINE_CFI_TYPE(cfi_bpf_subprog_hash, __bpf_callback_fn);
| ^
| int
arch/riscv/kernel/cfi.c:89:17: error: a parameter list without types is only allowed in a function definition
89 | DEFINE_CFI_TYPE(cfi_bpf_subprog_hash, __bpf_callback_fn);
| ^
4 errors generated.
vim +/int +85 arch/riscv/kernel/cfi.c
81
82 /* Must match bpf_func_t / DEFINE_BPF_PROG_RUN() */
83 extern unsigned int __bpf_prog_runX(const void *ctx,
84 const struct bpf_insn *insn);
> 85 DEFINE_CFI_TYPE(cfi_bpf_hash, __bpf_prog_runX);
86
--
0-DAY CI Kernel Test Service
https://github.com/intel/lkp-tests/wiki
On Mon, Jun 10, 2024 at 01:06:33PM -0500, Maxwell Bland wrote:
> From: Mark Rutland <[email protected]>
>
> Currently x86 and riscv open-code 4 instances of the same logic to
> define a u32 variable with the KCFI typeid of a given function.
>
> Replace the duplicate logic with a common macro.
>
> Signed-off-by: Mark Rutland <[email protected]>
This patch is missing your signed-off-by (the same with the second
patch). Since you are submitting it, you should also add yours in
addition to the author's s-o-b.
--
Catalin
On Tue, Jun 11, 2024 at 05:30:28PM GMT, Catalin Marinas wrote:
> This patch is missing your signed-off-by (the same with the second
> patch). Since you are submitting it, you should also add yours in
> addition to the author's s-o-b.
I see, thank you Catalin. I have also fixed the compiler errors.
Usually I would wait a week to resubmit, but since v5 took me a while to
get out the door, I've pushed a new version here:
https://lore.kernel.org/all/illfkwuxwq3adca2h4shibz2xub62kku3g2wte4sqp7xj7cwkb@ckn3qg7zxjuv/
Maxwell