When the verifier patches an ebpf program with bpf_patch_insn_data, it then calls adjust_insn_aux_data to make sure that insn_aux_data takes into account the newly inserted patch. Some of the data offset is pretty straightforward to deduce, it is for example the case for indirect_target, as any patch affecting indirect calls will systematically move the original instruction to the end of the new patch.
In order to introduce KASAN support for eBPF JIT, we need to mark any load/store instruction that accesses non-stack memory, but updating this new marking after a patch is not as straightforward as for indirect calls: the original BPF_ST/BPF_STX/BPF_LDX can be at the beginning, at the end or somewhere in the middle of the new patch: we then need some additional info to properly update this marking. Add a new parameter to bpf_patch_insn_data and adjust_insn_aux_data to convey the info about the new location in the patch of the original instruction. This info does not always make sense depending on the generated patch (eg, some bpf helpers being inlined by the verifier, and so turned into multiple new instructions without any remaining BPF_CALL), the parameter can then be set to -1. It will be used in next patches to properly handle insn_aux_data adjustment for the new KASAN instrumentation Signed-off-by: Alexis Lothoré (eBPF Foundation) <[email protected]> --- Changes in v4: - define insn_off_in_patch inside convert_ctx_accesses main loop to avoid old value leakage Changes in v3: - new patch --- include/linux/filter.h | 10 +++-- kernel/bpf/core.c | 2 +- kernel/bpf/fixups.c | 105 +++++++++++++++++++++++++++++++++---------------- 3 files changed, 79 insertions(+), 38 deletions(-) diff --git a/include/linux/filter.h b/include/linux/filter.h index 14acb2455746..1ebcd247ef4e 100644 --- a/include/linux/filter.h +++ b/include/linux/filter.h @@ -1210,13 +1210,17 @@ struct bpf_prog *bpf_patch_insn_single(struct bpf_prog *prog, u32 off, #ifdef CONFIG_BPF_SYSCALL struct bpf_prog *bpf_patch_insn_data(struct bpf_verifier_env *env, u32 off, - const struct bpf_insn *patch, u32 len); + const struct bpf_insn *patch, u32 len, + s32 insn_off_in_patch); struct bpf_insn_aux_data *bpf_dup_insn_aux_data(struct bpf_verifier_env *env); void bpf_restore_insn_aux_data(struct bpf_verifier_env *env, struct bpf_insn_aux_data *orig_insn_aux); #else -static inline struct bpf_prog *bpf_patch_insn_data(struct bpf_verifier_env *env, u32 off, - const struct bpf_insn *patch, u32 len) +static inline struct bpf_prog *bpf_patch_insn_data(struct bpf_verifier_env *env, + u32 off, + const struct bpf_insn *patch, + u32 len, + s32 insn_off_in_patch) { return ERR_PTR(-ENOTSUPP); } diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c index 6e19a030da6f..1e18b999f1b5 100644 --- a/kernel/bpf/core.c +++ b/kernel/bpf/core.c @@ -1598,7 +1598,7 @@ struct bpf_prog *bpf_jit_blind_constants(struct bpf_verifier_env *env, struct bp continue; if (env) - tmp = bpf_patch_insn_data(env, i, insn_buff, rewritten); + tmp = bpf_patch_insn_data(env, i, insn_buff, rewritten, rewritten - 1); else tmp = bpf_patch_insn_single(clone, i, insn_buff, rewritten); diff --git a/kernel/bpf/fixups.c b/kernel/bpf/fixups.c index 12a8a4eb757f..f515fc9669ce 100644 --- a/kernel/bpf/fixups.c +++ b/kernel/bpf/fixups.c @@ -157,7 +157,8 @@ static int get_callee_stack_depth(struct bpf_verifier_env *env, * [0, off) and [off, end) to new locations, so the patched range stays zero */ static void adjust_insn_aux_data(struct bpf_verifier_env *env, - struct bpf_prog *new_prog, u32 off, u32 cnt) + struct bpf_prog *new_prog, u32 off, u32 cnt, + s32 insn_off_in_patch) { struct bpf_insn_aux_data *data = env->insn_aux_data; struct bpf_insn *insn = new_prog->insnsi; @@ -245,7 +246,8 @@ static void adjust_poke_descs(struct bpf_prog *prog, u32 off, u32 len) } struct bpf_prog *bpf_patch_insn_data(struct bpf_verifier_env *env, u32 off, - const struct bpf_insn *patch, u32 len) + const struct bpf_insn *patch, u32 len, + s32 insn_off_in_patch) { struct bpf_prog *new_prog; struct bpf_insn_aux_data *new_data = NULL; @@ -269,7 +271,7 @@ struct bpf_prog *bpf_patch_insn_data(struct bpf_verifier_env *env, u32 off, env->insn_aux_data[off].orig_idx); return NULL; } - adjust_insn_aux_data(env, new_prog, off, len); + adjust_insn_aux_data(env, new_prog, off, len, insn_off_in_patch); adjust_subprog_starts(env, off, len); adjust_insn_arrays(env, off, len); adjust_poke_descs(new_prog, off, len); @@ -668,7 +670,7 @@ int bpf_opt_subreg_zext_lo32_rnd_hi32(struct bpf_verifier_env *env, patch = zext_patch; patch_len = 2; apply_patch_buffer: - new_prog = bpf_patch_insn_data(env, adj_idx, patch, patch_len); + new_prog = bpf_patch_insn_data(env, adj_idx, patch, patch_len, 0); if (!new_prog) return -ENOMEM; env->prog = new_prog; @@ -713,7 +715,7 @@ int bpf_convert_ctx_accesses(struct bpf_verifier_env *env) insn_buf[cnt++] = BPF_STX_MEM(BPF_DW, BPF_REG_FP, BPF_REG_1, -subprogs[0].stack_depth); insn_buf[cnt++] = env->prog->insnsi[0]; - new_prog = bpf_patch_insn_data(env, 0, insn_buf, cnt); + new_prog = bpf_patch_insn_data(env, 0, insn_buf, cnt, 1); if (!new_prog) return -ENOMEM; env->prog = new_prog; @@ -736,7 +738,8 @@ int bpf_convert_ctx_accesses(struct bpf_verifier_env *env) verifier_bug(env, "prologue is too long"); return -EFAULT; } else if (cnt) { - new_prog = bpf_patch_insn_data(env, 0, insn_buf, cnt); + new_prog = bpf_patch_insn_data(env, 0, insn_buf, cnt, + cnt - 1); if (!new_prog) return -ENOMEM; @@ -759,6 +762,7 @@ int bpf_convert_ctx_accesses(struct bpf_verifier_env *env) for (i = 0; i < insn_cnt; i++, insn++) { bpf_convert_ctx_access_t convert_ctx_access; + s32 insn_off_in_patch = -1; u8 mode; if (env->insn_aux_data[i + delta].nospec) { @@ -768,7 +772,8 @@ int bpf_convert_ctx_accesses(struct bpf_verifier_env *env) *patch++ = BPF_ST_NOSPEC(); *patch++ = *insn; cnt = patch - insn_buf; - new_prog = bpf_patch_insn_data(env, i + delta, insn_buf, cnt); + new_prog = bpf_patch_insn_data(env, i + delta, insn_buf, + cnt, cnt - 1); if (!new_prog) return -ENOMEM; @@ -841,7 +846,8 @@ int bpf_convert_ctx_accesses(struct bpf_verifier_env *env) *patch++ = *insn; *patch++ = BPF_ST_NOSPEC(); cnt = patch - insn_buf; - new_prog = bpf_patch_insn_data(env, i + delta, insn_buf, cnt); + new_prog = bpf_patch_insn_data(env, i + delta, insn_buf, + cnt, 0); if (!new_prog) return -ENOMEM; @@ -856,16 +862,20 @@ int bpf_convert_ctx_accesses(struct bpf_verifier_env *env) if (!ops->convert_ctx_access) continue; convert_ctx_access = ops->convert_ctx_access; + insn_off_in_patch = 0; break; case PTR_TO_SOCKET: case PTR_TO_SOCK_COMMON: convert_ctx_access = bpf_sock_convert_ctx_access; + insn_off_in_patch = 0; break; case PTR_TO_TCP_SOCK: convert_ctx_access = bpf_tcp_sock_convert_ctx_access; + insn_off_in_patch = 0; break; case PTR_TO_XDP_SOCK: convert_ctx_access = bpf_xdp_sock_convert_ctx_access; + insn_off_in_patch = 0; break; case PTR_TO_BTF_ID: case PTR_TO_BTF_ID | PTR_UNTRUSTED: @@ -971,7 +981,8 @@ int bpf_convert_ctx_accesses(struct bpf_verifier_env *env) size * 8, 0); patch_insn_buf: - new_prog = bpf_patch_insn_data(env, i + delta, insn_buf, cnt); + new_prog = bpf_patch_insn_data(env, i + delta, insn_buf, cnt, + insn_off_in_patch); if (!new_prog) return -ENOMEM; @@ -1464,7 +1475,7 @@ static int add_hidden_subprog(struct bpf_verifier_env *env, struct bpf_insn *pat * ones for the hidden subprog. Hence all of the adjustment operations * in bpf_patch_insn_data are no-ops. */ - prog = bpf_patch_insn_data(env, env->prog->len - 1, patch, len); + prog = bpf_patch_insn_data(env, env->prog->len - 1, patch, len, -1); if (!prog) return -ENOMEM; env->prog = prog; @@ -1549,7 +1560,8 @@ int bpf_do_misc_fixups(struct bpf_verifier_env *env) cnt = patch - insn_buf; - new_prog = bpf_patch_insn_data(env, i + delta, insn_buf, cnt); + new_prog = bpf_patch_insn_data(env, i + delta, insn_buf, + cnt, 0); if (!new_prog) return -ENOMEM; @@ -1569,6 +1581,7 @@ int bpf_do_misc_fixups(struct bpf_verifier_env *env) bool is_sdiv = isdiv && insn->off == 1; bool is_smod = !isdiv && insn->off == 1; struct bpf_insn *patch = insn_buf; + s32 insn_off_in_patch; if (is_sdiv) { /* [R,W]x sdiv 0 -> 0 @@ -1595,6 +1608,7 @@ int bpf_do_misc_fixups(struct bpf_verifier_env *env) *patch++ = BPF_JMP_IMM(BPF_JA, 0, 0, 1); *patch++ = *insn; cnt = patch - insn_buf; + insn_off_in_patch = cnt - 1; } else if (is_smod) { /* [R,W]x mod 0 -> [R,W]x */ /* [R,W]x mod -1 -> 0 */ @@ -1611,6 +1625,7 @@ int bpf_do_misc_fixups(struct bpf_verifier_env *env) *patch++ = BPF_MOV32_IMM(insn->dst_reg, 0); *patch++ = BPF_JMP_IMM(BPF_JA, 0, 0, 1); *patch++ = *insn; + insn_off_in_patch = patch - insn_buf - 1; if (!is64) { *patch++ = BPF_JMP_IMM(BPF_JA, 0, 0, 1); @@ -1626,12 +1641,14 @@ int bpf_do_misc_fixups(struct bpf_verifier_env *env) *patch++ = BPF_JMP_IMM(BPF_JA, 0, 0, 1); *patch++ = *insn; cnt = patch - insn_buf; + insn_off_in_patch = cnt - 1; } else { /* [R,W]x mod 0 -> [R,W]x */ *patch++ = BPF_RAW_INSN((is64 ? BPF_JMP : BPF_JMP32) | BPF_JEQ | BPF_K, insn->src_reg, 0, 1 + (is64 ? 0 : 1), 0); *patch++ = *insn; + insn_off_in_patch = patch - insn_buf - 1; if (!is64) { *patch++ = BPF_JMP_IMM(BPF_JA, 0, 0, 1); @@ -1640,7 +1657,8 @@ int bpf_do_misc_fixups(struct bpf_verifier_env *env) cnt = patch - insn_buf; } - new_prog = bpf_patch_insn_data(env, i + delta, insn_buf, cnt); + new_prog = bpf_patch_insn_data(env, i + delta, insn_buf, + cnt, insn_off_in_patch); if (!new_prog) return -ENOMEM; @@ -1656,6 +1674,7 @@ int bpf_do_misc_fixups(struct bpf_verifier_env *env) BPF_MODE(insn->code) == BPF_PROBE_MEMSX)) { struct bpf_insn *patch = insn_buf; u64 uaddress_limit = bpf_arch_uaddress_limit(); + s32 insn_off_in_patch; if (!uaddress_limit) goto next_insn; @@ -1666,11 +1685,13 @@ int bpf_do_misc_fixups(struct bpf_verifier_env *env) *patch++ = BPF_ALU64_IMM(BPF_RSH, BPF_REG_AX, 32); *patch++ = BPF_JMP_IMM(BPF_JLE, BPF_REG_AX, uaddress_limit >> 32, 2); *patch++ = *insn; + insn_off_in_patch = patch - insn_buf - 1; *patch++ = BPF_JMP_IMM(BPF_JA, 0, 0, 1); *patch++ = BPF_MOV64_IMM(insn->dst_reg, 0); cnt = patch - insn_buf; - new_prog = bpf_patch_insn_data(env, i + delta, insn_buf, cnt); + new_prog = bpf_patch_insn_data(env, i + delta, insn_buf, + cnt, insn_off_in_patch); if (!new_prog) return -ENOMEM; @@ -1690,7 +1711,8 @@ int bpf_do_misc_fixups(struct bpf_verifier_env *env) return -EFAULT; } - new_prog = bpf_patch_insn_data(env, i + delta, insn_buf, cnt); + new_prog = bpf_patch_insn_data(env, i + delta, insn_buf, + cnt, 0); if (!new_prog) return -ENOMEM; @@ -1743,7 +1765,8 @@ int bpf_do_misc_fixups(struct bpf_verifier_env *env) *patch++ = BPF_ALU64_IMM(BPF_MUL, off_reg, -1); cnt = patch - insn_buf; - new_prog = bpf_patch_insn_data(env, i + delta, insn_buf, cnt); + new_prog = bpf_patch_insn_data(env, i + delta, insn_buf, + cnt, -1); if (!new_prog) return -ENOMEM; @@ -1788,7 +1811,8 @@ int bpf_do_misc_fixups(struct bpf_verifier_env *env) insn_buf[6] = BPF_STX_MEM(BPF_DW, BPF_REG_10, BPF_REG_AX, stack_off_cnt); cnt = 7; - new_prog = bpf_patch_insn_data(env, i + delta, insn_buf, cnt); + new_prog = bpf_patch_insn_data(env, i + delta, insn_buf, + cnt, 3); if (!new_prog) return -ENOMEM; @@ -1809,7 +1833,8 @@ int bpf_do_misc_fixups(struct bpf_verifier_env *env) insn_buf[3] = BPF_STX_MEM(BPF_DW, BPF_REG_10, BPF_REG_AX, stack_off); cnt = 4; - new_prog = bpf_patch_insn_data(env, i + delta, insn_buf, cnt); + new_prog = bpf_patch_insn_data(env, i + delta, insn_buf, + cnt, 2); if (!new_prog) return -ENOMEM; @@ -1830,7 +1855,8 @@ int bpf_do_misc_fixups(struct bpf_verifier_env *env) if (cnt == 0) goto next_insn; - new_prog = bpf_patch_insn_data(env, i + delta, insn_buf, cnt); + new_prog = bpf_patch_insn_data(env, i + delta, insn_buf, + cnt, cnt - 1); if (!new_prog) return -ENOMEM; @@ -1915,7 +1941,8 @@ int bpf_do_misc_fixups(struct bpf_verifier_env *env) map)->index_mask); insn_buf[2] = *insn; cnt = 3; - new_prog = bpf_patch_insn_data(env, i + delta, insn_buf, cnt); + new_prog = bpf_patch_insn_data(env, i + delta, insn_buf, + cnt, cnt - 1); if (!new_prog) return -ENOMEM; @@ -1948,7 +1975,8 @@ int bpf_do_misc_fixups(struct bpf_verifier_env *env) insn_buf[2] = *insn; cnt = 3; - new_prog = bpf_patch_insn_data(env, i + delta, insn_buf, cnt); + new_prog = bpf_patch_insn_data(env, i + delta, insn_buf, + cnt, cnt - 1); if (!new_prog) return -ENOMEM; @@ -1967,7 +1995,8 @@ int bpf_do_misc_fixups(struct bpf_verifier_env *env) insn_buf[1] = *insn; cnt = 2; - new_prog = bpf_patch_insn_data(env, i + delta, insn_buf, cnt); + new_prog = bpf_patch_insn_data(env, i + delta, insn_buf, + cnt, cnt - 1); if (!new_prog) return -ENOMEM; @@ -2007,8 +2036,8 @@ int bpf_do_misc_fixups(struct bpf_verifier_env *env) return -EFAULT; } - new_prog = bpf_patch_insn_data(env, i + delta, - insn_buf, cnt); + new_prog = bpf_patch_insn_data( + env, i + delta, insn_buf, cnt, -1); if (!new_prog) return -ENOMEM; @@ -2091,7 +2120,7 @@ int bpf_do_misc_fixups(struct bpf_verifier_env *env) cnt = 3; new_prog = bpf_patch_insn_data(env, i + delta, insn_buf, - cnt); + cnt, -1); if (!new_prog) return -ENOMEM; @@ -2119,7 +2148,8 @@ int bpf_do_misc_fixups(struct bpf_verifier_env *env) insn_buf[0] = BPF_ALU32_REG(BPF_XOR, BPF_REG_0, BPF_REG_0); cnt = 1; #endif - new_prog = bpf_patch_insn_data(env, i + delta, insn_buf, cnt); + new_prog = bpf_patch_insn_data(env, i + delta, insn_buf, + cnt, -1); if (!new_prog) return -ENOMEM; @@ -2137,7 +2167,8 @@ int bpf_do_misc_fixups(struct bpf_verifier_env *env) insn_buf[2] = BPF_LDX_MEM(BPF_DW, BPF_REG_0, BPF_REG_0, 0); cnt = 3; - new_prog = bpf_patch_insn_data(env, i + delta, insn_buf, cnt); + new_prog = bpf_patch_insn_data(env, i + delta, insn_buf, + cnt, -1); if (!new_prog) return -ENOMEM; @@ -2171,7 +2202,8 @@ int bpf_do_misc_fixups(struct bpf_verifier_env *env) insn_buf[cnt++] = BPF_JMP_A(1); insn_buf[cnt++] = BPF_MOV64_IMM(BPF_REG_0, -EINVAL); - new_prog = bpf_patch_insn_data(env, i + delta, insn_buf, cnt); + new_prog = bpf_patch_insn_data(env, i + delta, insn_buf, + cnt, -1); if (!new_prog) return -ENOMEM; @@ -2203,7 +2235,8 @@ int bpf_do_misc_fixups(struct bpf_verifier_env *env) cnt = 1; } - new_prog = bpf_patch_insn_data(env, i + delta, insn_buf, cnt); + new_prog = bpf_patch_insn_data(env, i + delta, insn_buf, + cnt, -1); if (!new_prog) return -ENOMEM; @@ -2229,7 +2262,8 @@ int bpf_do_misc_fixups(struct bpf_verifier_env *env) cnt = 2; } - new_prog = bpf_patch_insn_data(env, i + delta, insn_buf, cnt); + new_prog = bpf_patch_insn_data(env, i + delta, insn_buf, + cnt, -1); if (!new_prog) return -ENOMEM; @@ -2245,7 +2279,8 @@ int bpf_do_misc_fixups(struct bpf_verifier_env *env) /* Load IP address from ctx - 16 */ insn_buf[0] = BPF_LDX_MEM(BPF_DW, BPF_REG_0, BPF_REG_1, -16); - new_prog = bpf_patch_insn_data(env, i + delta, insn_buf, 1); + new_prog = bpf_patch_insn_data(env, i + delta, insn_buf, + 1, -1); if (!new_prog) return -ENOMEM; @@ -2300,7 +2335,8 @@ int bpf_do_misc_fixups(struct bpf_verifier_env *env) insn_buf[10] = BPF_MOV64_IMM(BPF_REG_0, -ENOENT); cnt = 11; - new_prog = bpf_patch_insn_data(env, i + delta, insn_buf, cnt); + new_prog = bpf_patch_insn_data(env, i + delta, insn_buf, + cnt, -1); if (!new_prog) return -ENOMEM; @@ -2318,7 +2354,8 @@ int bpf_do_misc_fixups(struct bpf_verifier_env *env) insn_buf[1] = BPF_ATOMIC_OP(BPF_DW, BPF_XCHG, BPF_REG_1, BPF_REG_0, 0); cnt = 2; - new_prog = bpf_patch_insn_data(env, i + delta, insn_buf, cnt); + new_prog = bpf_patch_insn_data(env, i + delta, insn_buf, + cnt, -1); if (!new_prog) return -ENOMEM; @@ -2386,7 +2423,7 @@ int bpf_do_misc_fixups(struct bpf_verifier_env *env) /* Copy first actual insn to preserve it */ insn_buf[cnt++] = env->prog->insnsi[subprog_start]; - new_prog = bpf_patch_insn_data(env, subprog_start, insn_buf, cnt); + new_prog = bpf_patch_insn_data(env, subprog_start, insn_buf, cnt, cnt - 1); if (!new_prog) return -ENOMEM; env->prog = prog = new_prog; @@ -2485,7 +2522,7 @@ static struct bpf_prog *inline_bpf_loop(struct bpf_verifier_env *env, insn_buf[cnt++] = BPF_LDX_MEM(BPF_DW, BPF_REG_8, BPF_REG_10, r8_offset); *total_cnt = cnt; - new_prog = bpf_patch_insn_data(env, position, insn_buf, cnt); + new_prog = bpf_patch_insn_data(env, position, insn_buf, cnt, -1); if (!new_prog) return new_prog; -- 2.54.0
