The verifier sometimes makes mistakes[1][2] that may be exploited to
achieve arbitrary read/write. Currently, syzbot is continuously testing
bpf, and can find memory issues in bpf syscalls, but it can hardly find
mischecking/bugs in the verifier. We need runtime checks like KASAN in
BPF programs for this. This patch series implements address sanitize
in jited BPF progs for testing purpose, so that tools like syzbot can
find interesting bugs in the verifier automatically by, if possible,
generating and executing BPF programs that bypass the verifier but have
memory issues, then triggering this sanitizing.
The idea is to dispatch read/write addr of a BPF program to the kernel
functions that are instrumented by KASAN, to achieve indirect checking.
Indirect checking is adopted because this is much simple, instrument
direct checking like compilers makes the jit much more complex. The
main step is: back up all the scratch regs to extend BPF prog stack,
store addr to R1, and then insert the checking function before load
or store insns, during bpf_misc_fixup(). The stack size of BPF progs
is extended by 64 bytes in this mode, to backup R1~R5 to make sure
the checking funcs won't corrupt regs states. An extra Kconfig option
is used to enable this, so normal use case won't be impacted at all.
Also, not all ldx/stx/st are instrumented. Insns rewrote by other fixup
or conversion passes that use BPF_REG_AX are skipped, because that
conflicts with us; insns whose access addr is specified by R10 are also
skipped because they are trivial to verify.
Patch1 sanitizes st/stx insns, and Patch2 sanitizes ldx insns, Patch3 adds
selftests for instrumentation in each possible case, and all new/existing
selftests for the verifier can pass. Also, a BPF prog that also exploits
CVE-2022-23222 to achieve OOB read is provided[3], this can be perfertly
captured with this patch series.
[1] http://bit.do/CVE-2021-3490
[2] http://bit.do/CVE-2022-23222
[3] OOB-read: https://pastebin.com/raw/Ee1Cw492
v1 -> v2:
remove changes to JIT completely, backup regs to extended stack.
v2 -> v3:
fix missing-prototypes warning report by kernel test bot.
simplify regs backing up and rewrite corresponding selftests.
Hao Sun (3):
bpf: Sanitize STX/ST in jited BPF progs with KASAN
bpf: Sanitize LDX in jited BPF progs with KASAN
selftests/bpf: Add tests for LDX/STX/ST sanitize
kernel/bpf/Kconfig | 13 +
kernel/bpf/verifier.c | 173 ++++++++++
.../selftests/bpf/verifier/sanitize_st_ldx.c | 317 ++++++++++++++++++
3 files changed, 503 insertions(+)
create mode 100644 tools/testing/selftests/bpf/verifier/sanitize_st_ldx.c
base-commit: 2b3e8f6f5b939ceeb2e097339bf78ebaaf11dfe9
--
2.38.1
Add tests for LDX/STX/ST instrumentation in each possible case.
Five cases for STX/ST, nice cases for LDX. All new/existing
selftests can pass.
A slab-out-of-bounds read report is also availble, which is
achieved by exploiting CVE-2022-23222 and can be reproduced
in Linux v5.10: https://pastebin.com/raw/Ee1Cw492
Signed-off-by: Hao Sun <[email protected]>
---
.../selftests/bpf/verifier/sanitize_st_ldx.c | 317 ++++++++++++++++++
1 file changed, 317 insertions(+)
create mode 100644 tools/testing/selftests/bpf/verifier/sanitize_st_ldx.c
diff --git a/tools/testing/selftests/bpf/verifier/sanitize_st_ldx.c b/tools/testing/selftests/bpf/verifier/sanitize_st_ldx.c
new file mode 100644
index 000000000000..dfd53f97eb95
--- /dev/null
+++ b/tools/testing/selftests/bpf/verifier/sanitize_st_ldx.c
@@ -0,0 +1,317 @@
+#ifdef CONFIG_BPF_PROG_KASAN
+
+#define __BACKUP_REG(n) \
+ BPF_STX_MEM(BPF_DW, BPF_REG_10, BPF_REG_##n, INSN_OFF_MASK)
+#define BACKUP_SCRATCH_REGS \
+ __BACKUP_REG(1), __BACKUP_REG(2), __BACKUP_REG(3), __BACKUP_REG(4), \
+ __BACKUP_REG(5)
+
+#define __RESTORE_REG(n) \
+ BPF_LDX_MEM(BPF_DW, BPF_REG_##n, BPF_REG_10, INSN_OFF_MASK)
+#define RESTORE_SCRATCH_REGS \
+ __RESTORE_REG(1), __RESTORE_REG(2), __RESTORE_REG(3), \
+ __RESTORE_REG(4), __RESTORE_REG(5)
+
+{
+ "sanitize stx: dst is R1, off is zero",
+ .insns = {
+ BPF_MOV64_REG(BPF_REG_1, BPF_REG_10),
+ BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, -8),
+ BPF_ST_MEM(BPF_DW, BPF_REG_1, 0, 1),
+ BPF_LDX_MEM(BPF_DW, BPF_REG_0, BPF_REG_1, 0),
+ BPF_JMP_IMM(BPF_JEQ, BPF_REG_0, 1, 1),
+ BPF_MOV64_IMM(BPF_REG_0, 2),
+ BPF_EXIT_INSN(),
+ },
+ .result = ACCEPT,
+ .retval = 1,
+ .expected_insns = {
+ BACKUP_SCRATCH_REGS,
+ BPF_MOV64_REG(MAX_BPF_REG, BPF_REG_0),
+ BPF_EMIT_CALL(INSN_IMM_MASK),
+ RESTORE_SCRATCH_REGS,
+ BPF_MOV64_REG(BPF_REG_0, MAX_BPF_REG),
+ BPF_ST_MEM(BPF_DW, BPF_REG_1, 0, 1),
+ },
+},
+{
+ "sanitize stx: dst is R1",
+ .insns = {
+ BPF_MOV64_REG(BPF_REG_1, BPF_REG_10),
+ BPF_ST_MEM(BPF_DW, BPF_REG_1, -8, 1),
+ BPF_LDX_MEM(BPF_DW, BPF_REG_1, BPF_REG_1, -8),
+ BPF_JMP_IMM(BPF_JEQ, BPF_REG_1, 1, 2),
+ BPF_MOV64_IMM(BPF_REG_0, 2),
+ BPF_EXIT_INSN(),
+ BPF_MOV64_REG(BPF_REG_0, BPF_REG_1),
+ BPF_EXIT_INSN(),
+ },
+ .result = ACCEPT,
+ .retval = 1,
+ .expected_insns = {
+ BACKUP_SCRATCH_REGS,
+ BPF_MOV64_REG(MAX_BPF_REG, BPF_REG_0),
+ BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, -8),
+ BPF_EMIT_CALL(INSN_IMM_MASK),
+ RESTORE_SCRATCH_REGS,
+ BPF_MOV64_REG(BPF_REG_0, MAX_BPF_REG),
+ BPF_ST_MEM(BPF_DW, BPF_REG_1, -8, 1),
+ },
+},
+{
+ "sanitize stx: dst is other regs, off is zero",
+ .insns = {
+ BPF_MOV64_REG(BPF_REG_2, BPF_REG_10),
+ BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -8),
+ BPF_ST_MEM(BPF_DW, BPF_REG_2, 0, 1),
+ BPF_LDX_MEM(BPF_DW, BPF_REG_0, BPF_REG_2, 0),
+ BPF_JMP_IMM(BPF_JEQ, BPF_REG_0, 1, 1),
+ BPF_MOV64_IMM(BPF_REG_0, 2),
+ BPF_EXIT_INSN(),
+ },
+ .result = ACCEPT,
+ .retval = 1,
+ .expected_insns = {
+ BACKUP_SCRATCH_REGS,
+ BPF_MOV64_REG(MAX_BPF_REG, BPF_REG_0),
+ BPF_MOV64_REG(BPF_REG_1, BPF_REG_2),
+ BPF_EMIT_CALL(INSN_IMM_MASK),
+ RESTORE_SCRATCH_REGS,
+ BPF_MOV64_REG(BPF_REG_0, MAX_BPF_REG),
+ BPF_ST_MEM(BPF_DW, BPF_REG_2, 0, 1),
+ },
+},
+{
+ "sanitize stx: dst is other regs",
+ .insns = {
+ BPF_MOV64_REG(BPF_REG_2, BPF_REG_10),
+ BPF_ST_MEM(BPF_DW, BPF_REG_2, -8, 1),
+ BPF_LDX_MEM(BPF_DW, BPF_REG_0, BPF_REG_2, -8),
+ BPF_JMP_IMM(BPF_JEQ, BPF_REG_0, 1, 1),
+ BPF_MOV64_IMM(BPF_REG_0, 2),
+ BPF_EXIT_INSN(),
+ },
+ .result = ACCEPT,
+ .retval = 1,
+ .expected_insns = {
+ BACKUP_SCRATCH_REGS,
+ BPF_MOV64_REG(MAX_BPF_REG, BPF_REG_0),
+ BPF_MOV64_REG(BPF_REG_1, BPF_REG_2),
+ BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, -8),
+ BPF_EMIT_CALL(INSN_IMM_MASK),
+ RESTORE_SCRATCH_REGS,
+ BPF_MOV64_REG(BPF_REG_0, MAX_BPF_REG),
+ BPF_ST_MEM(BPF_DW, BPF_REG_2, -8, 1),
+ },
+},
+{
+ "sanitize stx: dst is R10",
+ .insns = {
+ BPF_ST_MEM(BPF_DW, BPF_REG_10, -8, 1),
+ BPF_LDX_MEM(BPF_DW, BPF_REG_0, BPF_REG_10, -8),
+ BPF_JMP_IMM(BPF_JEQ, BPF_REG_0, 1, 1),
+ BPF_MOV64_IMM(BPF_REG_0, 2),
+ BPF_EXIT_INSN(),
+ },
+ .result = ACCEPT,
+ .retval = 1,
+ .unexpected_insns = {
+ BPF_EMIT_CALL(INSN_IMM_MASK),
+ },
+},
+{
+ "sanitize ldx: src is R1, dst is R0, off is zero",
+ .insns = {
+ BPF_MOV64_REG(BPF_REG_1, BPF_REG_10),
+ BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, -8),
+ BPF_ST_MEM(BPF_DW, BPF_REG_1, 0, 1),
+ BPF_LDX_MEM(BPF_DW, BPF_REG_0, BPF_REG_1, 0),
+ BPF_JMP_IMM(BPF_JEQ, BPF_REG_0, 1, 1),
+ BPF_MOV64_IMM(BPF_REG_0, 2),
+ BPF_EXIT_INSN(),
+ },
+ .result = ACCEPT,
+ .retval = 1,
+ .expected_insns = {
+ BACKUP_SCRATCH_REGS,
+ BPF_EMIT_CALL(INSN_IMM_MASK),
+ RESTORE_SCRATCH_REGS,
+ BPF_LDX_MEM(BPF_DW, BPF_REG_0, BPF_REG_1, 0),
+ },
+},
+{
+ "sanitize ldx: src is R1, dst is R0",
+ .insns = {
+ BPF_MOV64_REG(BPF_REG_1, BPF_REG_10),
+ BPF_ST_MEM(BPF_DW, BPF_REG_1, -8, 1),
+ BPF_LDX_MEM(BPF_DW, BPF_REG_0, BPF_REG_1, -8),
+ BPF_JMP_IMM(BPF_JEQ, BPF_REG_0, 1, 1),
+ BPF_MOV64_IMM(BPF_REG_0, 2),
+ BPF_EXIT_INSN(),
+ },
+ .result = ACCEPT,
+ .retval = 1,
+ .expected_insns = {
+ BACKUP_SCRATCH_REGS,
+ BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, -8),
+ BPF_EMIT_CALL(INSN_IMM_MASK),
+ RESTORE_SCRATCH_REGS,
+ BPF_LDX_MEM(BPF_DW, BPF_REG_0, BPF_REG_1, -8),
+ },
+},
+{
+ "sanitize ldx: src is R1, dst is others, off is zero",
+ .insns = {
+ BPF_MOV64_REG(BPF_REG_1, BPF_REG_10),
+ BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, -8),
+ BPF_ST_MEM(BPF_DW, BPF_REG_1, 0, 1),
+ BPF_LDX_MEM(BPF_DW, BPF_REG_1, BPF_REG_1, 0),
+ BPF_JMP_IMM(BPF_JEQ, BPF_REG_1, 1, 2),
+ BPF_MOV64_IMM(BPF_REG_0, 2),
+ BPF_EXIT_INSN(),
+ BPF_MOV64_REG(BPF_REG_0, BPF_REG_1),
+ BPF_EXIT_INSN(),
+ },
+ .result = ACCEPT,
+ .retval = 1,
+ .expected_insns = {
+ BACKUP_SCRATCH_REGS,
+ BPF_MOV64_REG(MAX_BPF_REG, BPF_REG_0),
+ BPF_EMIT_CALL(INSN_IMM_MASK),
+ RESTORE_SCRATCH_REGS,
+ BPF_MOV64_REG(BPF_REG_0, MAX_BPF_REG),
+ BPF_LDX_MEM(BPF_DW, BPF_REG_1, BPF_REG_1, 0),
+ },
+},
+{
+ "sanitize ldx: src is R1, dst is others",
+ .insns = {
+ BPF_MOV64_REG(BPF_REG_1, BPF_REG_10),
+ BPF_ST_MEM(BPF_DW, BPF_REG_1, -8, 1),
+ BPF_LDX_MEM(BPF_DW, BPF_REG_1, BPF_REG_1, -8),
+ BPF_JMP_IMM(BPF_JEQ, BPF_REG_1, 1, 2),
+ BPF_MOV64_IMM(BPF_REG_0, 2),
+ BPF_EXIT_INSN(),
+ BPF_MOV64_REG(BPF_REG_0, BPF_REG_1),
+ BPF_EXIT_INSN(),
+ },
+ .result = ACCEPT,
+ .retval = 1,
+ .expected_insns = {
+ BACKUP_SCRATCH_REGS,
+ BPF_MOV64_REG(MAX_BPF_REG, BPF_REG_0),
+ BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, -8),
+ BPF_EMIT_CALL(INSN_IMM_MASK),
+ RESTORE_SCRATCH_REGS,
+ BPF_MOV64_REG(BPF_REG_0, MAX_BPF_REG),
+ BPF_LDX_MEM(BPF_DW, BPF_REG_1, BPF_REG_1, -8),
+ },
+},
+{
+ "sanitize ldx: src is others, dst is R0, off is zero",
+ .insns = {
+ BPF_MOV64_REG(BPF_REG_2, BPF_REG_10),
+ BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -8),
+ BPF_ST_MEM(BPF_DW, BPF_REG_2, 0, 1),
+ BPF_LDX_MEM(BPF_DW, BPF_REG_0, BPF_REG_2, 0),
+ BPF_JMP_IMM(BPF_JEQ, BPF_REG_0, 1, 1),
+ BPF_MOV64_IMM(BPF_REG_0, 2),
+ BPF_EXIT_INSN(),
+ },
+ .result = ACCEPT,
+ .retval = 1,
+ .expected_insns = {
+ BACKUP_SCRATCH_REGS,
+ BPF_MOV64_REG(BPF_REG_1, BPF_REG_2),
+ BPF_EMIT_CALL(INSN_IMM_MASK),
+ RESTORE_SCRATCH_REGS,
+ BPF_LDX_MEM(BPF_DW, BPF_REG_0, BPF_REG_2, 0),
+ },
+},
+{
+ "sanitize ldx: src is others, dst is R0",
+ .insns = {
+ BPF_MOV64_REG(BPF_REG_2, BPF_REG_10),
+ BPF_ST_MEM(BPF_DW, BPF_REG_2, -8, 1),
+ BPF_LDX_MEM(BPF_DW, BPF_REG_0, BPF_REG_2, -8),
+ BPF_JMP_IMM(BPF_JEQ, BPF_REG_0, 1, 1),
+ BPF_MOV64_IMM(BPF_REG_0, 2),
+ BPF_EXIT_INSN(),
+ },
+ .result = ACCEPT,
+ .retval = 1,
+ .expected_insns = {
+ BACKUP_SCRATCH_REGS,
+ BPF_MOV64_REG(BPF_REG_1, BPF_REG_2),
+ BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, -8),
+ BPF_EMIT_CALL(INSN_IMM_MASK),
+ RESTORE_SCRATCH_REGS,
+ BPF_LDX_MEM(BPF_DW, BPF_REG_0, BPF_REG_2, -8),
+ },
+},
+{
+ "sanitize ldx: src is others, dst is others, off is zero",
+ .insns = {
+ BPF_MOV64_REG(BPF_REG_2, BPF_REG_10),
+ BPF_ALU64_IMM(BPF_ADD, BPF_REG_2, -8),
+ BPF_ST_MEM(BPF_DW, BPF_REG_2, 0, 1),
+ BPF_LDX_MEM(BPF_DW, BPF_REG_1, BPF_REG_2, 0),
+ BPF_JMP_IMM(BPF_JEQ, BPF_REG_1, 1, 2),
+ BPF_MOV64_IMM(BPF_REG_0, 2),
+ BPF_EXIT_INSN(),
+ BPF_MOV64_REG(BPF_REG_0, BPF_REG_1),
+ BPF_EXIT_INSN(),
+ },
+ .result = ACCEPT,
+ .retval = 1,
+ .expected_insns = {
+ BACKUP_SCRATCH_REGS,
+ BPF_MOV64_REG(MAX_BPF_REG, BPF_REG_0),
+ BPF_MOV64_REG(BPF_REG_1, BPF_REG_2),
+ BPF_EMIT_CALL(INSN_IMM_MASK),
+ RESTORE_SCRATCH_REGS,
+ BPF_MOV64_REG(BPF_REG_0, MAX_BPF_REG),
+ BPF_LDX_MEM(BPF_DW, BPF_REG_1, BPF_REG_2, 0),
+ },
+},
+{
+ "sanitize ldx: src is others, dst is others",
+ .insns = {
+ BPF_MOV64_REG(BPF_REG_2, BPF_REG_10),
+ BPF_ST_MEM(BPF_DW, BPF_REG_2, -8, 1),
+ BPF_LDX_MEM(BPF_DW, BPF_REG_1, BPF_REG_2, -8),
+ BPF_JMP_IMM(BPF_JEQ, BPF_REG_1, 1, 2),
+ BPF_MOV64_IMM(BPF_REG_0, 2),
+ BPF_EXIT_INSN(),
+ BPF_MOV64_REG(BPF_REG_0, BPF_REG_1),
+ BPF_EXIT_INSN(),
+ },
+ .result = ACCEPT,
+ .retval = 1,
+ .expected_insns = {
+ BACKUP_SCRATCH_REGS,
+ BPF_MOV64_REG(MAX_BPF_REG, BPF_REG_0),
+ BPF_MOV64_REG(BPF_REG_1, BPF_REG_2),
+ BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, -8),
+ BPF_EMIT_CALL(INSN_IMM_MASK),
+ RESTORE_SCRATCH_REGS,
+ BPF_MOV64_REG(BPF_REG_0, MAX_BPF_REG),
+ BPF_LDX_MEM(BPF_DW, BPF_REG_1, BPF_REG_2, -8),
+ },
+},
+{
+ "sanitize LDX: SRC is R10",
+ .insns = {
+ BPF_ST_MEM(BPF_DW, BPF_REG_10, -8, 1),
+ BPF_LDX_MEM(BPF_DW, BPF_REG_0, BPF_REG_10, -8),
+ BPF_JMP_IMM(BPF_JEQ, BPF_REG_0, 1, 1),
+ BPF_MOV64_IMM(BPF_REG_0, 2),
+ BPF_EXIT_INSN(),
+ },
+ .result = ACCEPT,
+ .retval = 1,
+ .unexpected_insns = {
+ BPF_EMIT_CALL(INSN_IMM_MASK),
+ },
+},
+#endif /* CONFIG_BPF_PROG_KASAN */
--
2.38.1
Make the verifier sanitize LDX insns in jited BPF programs. Saved
all the scratch regs to the extended stack first, skip backing up
of R0 if it is the dst_reg, then save checking addr to R1. Finally
the checking funcs are inserted, and regs are restored then.
Signed-off-by: Hao Sun <[email protected]>
---
kernel/bpf/verifier.c | 60 +++++++++++++++++++++++++++++++++++++++++++
1 file changed, 60 insertions(+)
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 7a31fceee370..b3b6855a9756 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -15345,6 +15345,18 @@ BPF_ASAN_STORE(16);
BPF_ASAN_STORE(32);
BPF_ASAN_STORE(64);
+#define BPF_ASAN_LOAD(n) \
+ notrace u64 bpf_asan_load##n(u##n *addr); \
+ notrace u64 bpf_asan_load##n(u##n *addr) \
+ { \
+ return *addr; \
+ }
+
+BPF_ASAN_LOAD(8);
+BPF_ASAN_LOAD(16);
+BPF_ASAN_LOAD(32);
+BPF_ASAN_LOAD(64);
+
#endif
/* Do various post-verification rewrites in a single program pass.
@@ -15567,6 +15579,54 @@ static int do_misc_fixups(struct bpf_verifier_env *env)
insn = new_prog->insnsi + i + delta;
continue;
}
+
+ /* Sanitize LDX operation*/
+ if (BPF_CLASS(insn->code) == BPF_LDX) {
+ struct bpf_insn sanitize_fn;
+ struct bpf_insn *patch = &insn_buf[0];
+
+ if (in_patch_use_ax || insn->src_reg == BPF_REG_10)
+ continue;
+
+ switch (BPF_SIZE(insn->code)) {
+ case BPF_B:
+ sanitize_fn = BPF_EMIT_CALL(bpf_asan_load8);
+ break;
+ case BPF_H:
+ sanitize_fn = BPF_EMIT_CALL(bpf_asan_load16);
+ break;
+ case BPF_W:
+ sanitize_fn = BPF_EMIT_CALL(bpf_asan_load32);
+ break;
+ case BPF_DW:
+ sanitize_fn = BPF_EMIT_CALL(bpf_asan_load64);
+ break;
+ }
+
+ BACKUP_SCRATCH_REGS;
+ /* Skip R0, if it is dst but not src */
+ if (insn->dst_reg != BPF_REG_0 || insn->src_reg == BPF_REG_0)
+ *patch++ = BPF_MOV64_REG(BPF_REG_AX, BPF_REG_0);
+ if (insn->src_reg != BPF_REG_1)
+ *patch++ = BPF_MOV64_REG(BPF_REG_1, insn->src_reg);
+ if (insn->off != 0)
+ *patch++ = BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, insn->off);
+ *patch++ = sanitize_fn;
+ RESTORE_SCRATCH_REGS;
+ if (insn->dst_reg != BPF_REG_0 || insn->src_reg == BPF_REG_0)
+ *patch++ = BPF_MOV64_REG(BPF_REG_0, BPF_REG_AX);
+ *patch++ = *insn;
+ cnt = patch - insn_buf;
+
+ new_prog = bpf_patch_insn_data(env, i + delta, insn_buf, cnt);
+ if (!new_prog)
+ return -ENOMEM;
+
+ delta += cnt - 1;
+ env->prog = prog = new_prog;
+ insn = new_prog->insnsi + i + delta;
+ continue;
+ }
#endif
if (insn->code != (BPF_JMP | BPF_CALL))
--
2.38.1
Make the verifier sanitize STX/ST insns in jited BPF programs
by dispatching addr to kernel functions that are instrumented
by KASAN.
Only STX/ST insns that aren't in patches added by other passes
using REG_AX or dst_reg isn't R10 are sanitized. The former
confilicts with us, the latter are trivial for the verifier to
check, skip them to reduce the footprint.
The instrumentation is conducted in bpf_misc_fixup(). During it,
all the scratch regs are saved to extended stack to make sure
the checking functions can't corrupt them. Then, the addr to
check plus offset is saved to R1, R0 is backed up to REG_AX.
We extend stack size in this mode because we don't rely on any
verifier's knowledge about calculated stack size or the liveness
of each reg. The corresponding bpf_asan_storeN() is inserted
before store insn, and then regs are restored. The checking
functions are instrumented with KASAN and they simply write to
the target addr for certain bytes, KASAN conducts the actual
checking. An extra Kconfig is used to enable this, so normal
use case won't be impacted at all.
Signed-off-by: Hao Sun <[email protected]>
---
kernel/bpf/Kconfig | 13 +++++
kernel/bpf/verifier.c | 113 ++++++++++++++++++++++++++++++++++++++++++
2 files changed, 126 insertions(+)
diff --git a/kernel/bpf/Kconfig b/kernel/bpf/Kconfig
index 2dfe1079f772..d41e1d2d26f1 100644
--- a/kernel/bpf/Kconfig
+++ b/kernel/bpf/Kconfig
@@ -99,4 +99,17 @@ config BPF_LSM
If you are unsure how to answer this question, answer N.
+config BPF_PROG_KASAN
+ bool "Enable BPF Program Address Sanitize"
+ depends on BPF_JIT_ALWAYS_ON
+ depends on KASAN
+ help
+ Enables instrumentation on LDX/STX/ST insn to capture memory
+ access errors in BPF programs missed by the verifier.
+
+ The actual check is conducted by KASAN, this feature presents
+ certain overhead, and should be used mainly by testing purpose.
+
+ If you are unsure how to answer this question, answer N.
+
endmenu # "BPF subsystem"
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 6599d25dae38..7a31fceee370 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -15327,6 +15327,26 @@ static int fixup_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
return 0;
}
+#ifdef CONFIG_BPF_PROG_KASAN
+
+/* Those are functions instrumented with KASAN for actual sanitizing. */
+
+#define BPF_ASAN_STORE(n) \
+ notrace u64 bpf_asan_store##n(u##n *addr); \
+ notrace u64 bpf_asan_store##n(u##n *addr) \
+ { \
+ u##n ret = *addr; \
+ *addr = ret; \
+ return ret; \
+ }
+
+BPF_ASAN_STORE(8);
+BPF_ASAN_STORE(16);
+BPF_ASAN_STORE(32);
+BPF_ASAN_STORE(64);
+
+#endif
+
/* Do various post-verification rewrites in a single program pass.
* These rewrites simplify JIT and interpreter implementations.
*/
@@ -15344,6 +15364,9 @@ static int do_misc_fixups(struct bpf_verifier_env *env)
struct bpf_prog *new_prog;
struct bpf_map *map_ptr;
int i, ret, cnt, delta = 0;
+#ifdef CONFIG_BPF_PROG_KASAN
+ bool in_patch_use_ax = false;
+#endif
for (i = 0; i < insn_cnt; i++, insn++) {
/* Make divide-by-zero exceptions impossible. */
@@ -15460,6 +15483,92 @@ static int do_misc_fixups(struct bpf_verifier_env *env)
continue;
}
+#ifdef CONFIG_BPF_PROG_KASAN
+
+/* With CONFIG_BPF_PROG_KASAN, we extend prog stack to MAX_BPF_STACK + 64
+ * to backup scratch regs before calling the sanitize functions, because
+ * we don't rely on verifier's knowledge about calculated stack size or
+ * liveness of each reg.
+ */
+#define __BACKUP_REG(n) \
+ *patch++ = BPF_STX_MEM(BPF_DW, BPF_REG_10, BPF_REG_##n, \
+ -(MAX_BPF_STACK + 8 * n))
+#define BACKUP_SCRATCH_REGS \
+ __BACKUP_REG(1); \
+ __BACKUP_REG(2); \
+ __BACKUP_REG(3); \
+ __BACKUP_REG(4); \
+ __BACKUP_REG(5)
+
+#define __RESTORE_REG(n) \
+ *patch++ = BPF_LDX_MEM(BPF_DW, BPF_REG_##n, BPF_REG_10, \
+ -(MAX_BPF_STACK + 8 * n))
+#define RESTORE_SCRATCH_REGS \
+ __RESTORE_REG(1); \
+ __RESTORE_REG(2); \
+ __RESTORE_REG(3); \
+ __RESTORE_REG(4); \
+ __RESTORE_REG(5)
+
+ /* Patches that use REG_AX confilict with us, skip it.
+ * This starts with first use of REG_AX, stops only when
+ * we see next ldx/stx/st insn with valid aux information.
+ */
+ aux = &env->insn_aux_data[i + delta];
+ if (in_patch_use_ax && (int)aux->ptr_type != 0)
+ in_patch_use_ax = false;
+ if (insn->dst_reg == BPF_REG_AX || insn->src_reg == BPF_REG_AX)
+ in_patch_use_ax = true;
+
+ /* Sanitize ST/STX operation. */
+ if (BPF_CLASS(insn->code) == BPF_ST ||
+ BPF_CLASS(insn->code) == BPF_STX) {
+ struct bpf_insn sanitize_fn;
+ struct bpf_insn *patch = &insn_buf[0];
+
+ /* Skip st/stx to R10, they're trivial to check. */
+ if (in_patch_use_ax || insn->dst_reg == BPF_REG_10 ||
+ BPF_MODE(insn->code) == BPF_NOSPEC)
+ continue;
+
+ switch (BPF_SIZE(insn->code)) {
+ case BPF_B:
+ sanitize_fn = BPF_EMIT_CALL(bpf_asan_store8);
+ break;
+ case BPF_H:
+ sanitize_fn = BPF_EMIT_CALL(bpf_asan_store16);
+ break;
+ case BPF_W:
+ sanitize_fn = BPF_EMIT_CALL(bpf_asan_store32);
+ break;
+ case BPF_DW:
+ sanitize_fn = BPF_EMIT_CALL(bpf_asan_store64);
+ break;
+ }
+
+ BACKUP_SCRATCH_REGS;
+ *patch++ = BPF_MOV64_REG(BPF_REG_AX, BPF_REG_0);
+ if (insn->dst_reg != BPF_REG_1)
+ *patch++ = BPF_MOV64_REG(BPF_REG_1, insn->dst_reg);
+ if (insn->off != 0)
+ *patch++ = BPF_ALU64_IMM(BPF_ADD, BPF_REG_1, insn->off);
+ *patch++ = sanitize_fn;
+ RESTORE_SCRATCH_REGS;
+ *patch++ = BPF_MOV64_REG(BPF_REG_0, BPF_REG_AX);
+ *patch++ = *insn;
+ cnt = patch - insn_buf;
+
+ new_prog = bpf_patch_insn_data(env, i + delta, insn_buf, cnt);
+ if (!new_prog)
+ return -ENOMEM;
+
+ delta += cnt - 1;
+ env->prog = prog = new_prog;
+ insn = new_prog->insnsi + i + delta;
+ continue;
+ }
+#endif
+
if (insn->code != (BPF_JMP | BPF_CALL))
continue;
if (insn->src_reg == BPF_PSEUDO_CALL)
@@ -15852,6 +15961,10 @@ static int do_misc_fixups(struct bpf_verifier_env *env)
}
}
+#ifdef CONFIG_BPF_PROG_KASAN
+ prog->aux->stack_depth = MAX_BPF_STACK + 64;
+#endif
+
sort_kfunc_descs_by_imm(env->prog);
return 0;
--
2.38.1
On Fri, Nov 25, 2022 at 08:29:09PM +0800, Hao Sun wrote:
> The verifier sometimes makes mistakes[1][2] that may be exploited to
> achieve arbitrary read/write. Currently, syzbot is continuously testing
> bpf, and can find memory issues in bpf syscalls, but it can hardly find
> mischecking/bugs in the verifier. We need runtime checks like KASAN in
> BPF programs for this. This patch series implements address sanitize
> in jited BPF progs for testing purpose, so that tools like syzbot can
> find interesting bugs in the verifier automatically by, if possible,
> generating and executing BPF programs that bypass the verifier but have
> memory issues, then triggering this sanitizing.
The above paragraph makes it sound that it's currently impossible to
use kasan with BPF. Which is confusing and incorrect statement.
kasan adds all the necessary instrumentation to BPF interpreter already
and syzbot can perform bug discovery.
syzbot runner should just disable JIT and run all progs via interpreter.
Adding all this logic to run JITed progs in kasan kernel is
just unnecessary complexity.
Alexei Starovoitov <[email protected]> 于2022年11月28日周一 08:38写道：
>
> On Fri, Nov 25, 2022 at 08:29:09PM +0800, Hao Sun wrote:
> > The verifier sometimes makes mistakes[1][2] that may be exploited to
> > achieve arbitrary read/write. Currently, syzbot is continuously testing
> > bpf, and can find memory issues in bpf syscalls, but it can hardly find
> > mischecking/bugs in the verifier. We need runtime checks like KASAN in
> > BPF programs for this. This patch series implements address sanitize
> > in jited BPF progs for testing purpose, so that tools like syzbot can
> > find interesting bugs in the verifier automatically by, if possible,
> > generating and executing BPF programs that bypass the verifier but have
> > memory issues, then triggering this sanitizing.
>
> The above paragraph makes it sound that it's currently impossible to
> use kasan with BPF. Which is confusing and incorrect statement.
> kasan adds all the necessary instrumentation to BPF interpreter already
> and syzbot can perform bug discovery.
> syzbot runner should just disable JIT and run all progs via interpreter.
> Adding all this logic to run JITed progs in kasan kernel is
> just unnecessary complexity.
Sorry for the confusion, I mean JITed BPF prog can't use KASAN currently,
maybe it should be called BPF_JITED_PROG_KASAN.
It's actually useful because JIT is used in most real cases for testing/fuzzing,
syzbot uses WITH_JIT_ALWAYS_ON[1][2]. For those tools, they may need
to run hundred times for each generated BPF prog to find interesting bugs in
the verifier, JIT makes it much faster. Also, bugs in JIT can be
missed if they're
disabled.
[1] http://bit.do/syzbot-bpf-config
[2] http://bit.do/syzbot-bpf-next-config
On Sun, Nov 27, 2022 at 5:41 PM Hao Sun <[email protected]> wrote:
>
> Alexei Starovoitov <[email protected]> 于2022年11月28日周一 08:38写道：
> >
> > On Fri, Nov 25, 2022 at 08:29:09PM +0800, Hao Sun wrote:
> > > The verifier sometimes makes mistakes[1][2] that may be exploited to
> > > achieve arbitrary read/write. Currently, syzbot is continuously testing
> > > bpf, and can find memory issues in bpf syscalls, but it can hardly find
> > > mischecking/bugs in the verifier. We need runtime checks like KASAN in
> > > BPF programs for this. This patch series implements address sanitize
> > > in jited BPF progs for testing purpose, so that tools like syzbot can
> > > find interesting bugs in the verifier automatically by, if possible,
> > > generating and executing BPF programs that bypass the verifier but have
> > > memory issues, then triggering this sanitizing.
> >
> > The above paragraph makes it sound that it's currently impossible to
> > use kasan with BPF. Which is confusing and incorrect statement.
> > kasan adds all the necessary instrumentation to BPF interpreter already
> > and syzbot can perform bug discovery.
> > syzbot runner should just disable JIT and run all progs via interpreter.
> > Adding all this logic to run JITed progs in kasan kernel is
> > just unnecessary complexity.
>
> Sorry for the confusion, I mean JITed BPF prog can't use KASAN currently,
> maybe it should be called BPF_JITED_PROG_KASAN.
>
> It's actually useful because JIT is used in most real cases for testing/fuzzing,
> syzbot uses WITH_JIT_ALWAYS_ON[1][2].
Just turn it off in syzbot. jit_always_on is a security feature
because of speculative execution bugs that can exploit
any in-kernel interpreter (not only bpf interpreter).
> For those tools, they may need
> to run hundred times for each generated BPF prog to find interesting bugs in
> the verifier, JIT makes it much faster.
Unlikely. With all the overhead of saving a bunch of regs,
restoring them and calling functions instead of direct load/store
such JITed code is probably running at the same speed as
interpreter.
Also syzbot generated progs are tiny.
Your oob reproducer is tiny too.
The speed of execution doesn't matter in such cases.
> Also, bugs in JIT can be
> missed if they're
> disabled.
Disagree. Replacing direct load/store with calls
doesn't improve JIT test coverage.
Also think long term. Beyond kasan there are various *sans
that instrument code differently. load/store may not be
the only insns that should be instrumented.
So hacking JITs either directly or via verifier isn't going
to scale.
Alexei Starovoitov <[email protected]> 于2022年11月28日周一 10:12写道：
>
> On Sun, Nov 27, 2022 at 5:41 PM Hao Sun <[email protected]> wrote:
> >
> > Alexei Starovoitov <[email protected]> 于2022年11月28日周一 08:38写道：
> > >
> > > On Fri, Nov 25, 2022 at 08:29:09PM +0800, Hao Sun wrote:
> > > > The verifier sometimes makes mistakes[1][2] that may be exploited to
> > > > achieve arbitrary read/write. Currently, syzbot is continuously testing
> > > > bpf, and can find memory issues in bpf syscalls, but it can hardly find
> > > > mischecking/bugs in the verifier. We need runtime checks like KASAN in
> > > > BPF programs for this. This patch series implements address sanitize
> > > > in jited BPF progs for testing purpose, so that tools like syzbot can
> > > > find interesting bugs in the verifier automatically by, if possible,
> > > > generating and executing BPF programs that bypass the verifier but have
> > > > memory issues, then triggering this sanitizing.
> > >
> > > The above paragraph makes it sound that it's currently impossible to
> > > use kasan with BPF. Which is confusing and incorrect statement.
> > > kasan adds all the necessary instrumentation to BPF interpreter already
> > > and syzbot can perform bug discovery.
> > > syzbot runner should just disable JIT and run all progs via interpreter.
> > > Adding all this logic to run JITed progs in kasan kernel is
> > > just unnecessary complexity.
> >
> > Sorry for the confusion, I mean JITed BPF prog can't use KASAN currently,
> > maybe it should be called BPF_JITED_PROG_KASAN.
> >
> > It's actually useful because JIT is used in most real cases for testing/fuzzing,
> > syzbot uses WITH_JIT_ALWAYS_ON[1][2].
>
> Just turn it off in syzbot. jit_always_on is a security feature
> because of speculative execution bugs that can exploit
> any in-kernel interpreter (not only bpf interpreter).
>
Will do that, thanks for the information.
> > For those tools, they may need
> > to run hundred times for each generated BPF prog to find interesting bugs in
> > the verifier, JIT makes it much faster.
>
> Unlikely. With all the overhead of saving a bunch of regs,
> restoring them and calling functions instead of direct load/store
> such JITed code is probably running at the same speed as
> interpreter.
> Also syzbot generated progs are tiny.
> Your oob reproducer is tiny too.
> The speed of execution doesn't matter in such cases.
>
Hard to tell which one is faster, since each execution of insn in the
interpreter requires a jmp.
But you're right, did not think about this, I guess randomly generated
progs that can pass the verifier are normally tiny, so the speed indeed
may not be an issue here.
> > Also, bugs in JIT can be
> > missed if they're
> > disabled.
>
> Disagree. Replacing direct load/store with calls
> doesn't improve JIT test coverage.
>
> Also think long term. Beyond kasan there are various *sans
> that instrument code differently. load/store may not be
> the only insns that should be instrumented.
> So hacking JITs either directly or via verifier isn't going
> to scale.
Right, just let those *sans instrument the interpreter is more scalable.
Thanks
Hao