A few patches do not expand in place, they prepend: gen_epilogue and gen_prologue splice their instructions in front of insn 0 in bpf_convert_ctx_accesses(), and bpf_do_misc_fixups() prepends the may_goto counter init at every subprog start. They copy the original instruction of 'tgt_idx' into the last slot of the patch buffer, so it now lives at tgt_idx + delta, and call adjust_jmp_off() to move direct branches from tgt_idx to tgt_idx + delta. Nothing does the same for indirect branches, so a BPF_MAP_TYPE_INSN_ARRAY slot that named tgt_idx keeps naming tgt_idx, which is now the first prepended instruction, and insn_aux_data[tgt_idx].indirect_target makes the JIT emit the landing pad there. Add a __bpf_patch_insn_data() variant and pass the BPF_PREPEND at the affected call-sites to fix up the delta. Fixes: 493d9e0d6083 ("bpf, x86: add support for indirect jumps") Fixes: 07ae6c130b46 ("bpf: Add helper to detect indirect jump targets") Reported-by: Nicholas Carlini Suggested-by: Nicholas Carlini Signed-off-by: Daniel Borkmann Acked-by: Anton Protopopov --- include/linux/bpf.h | 2 +- kernel/bpf/bpf_insn_array.c | 4 ++-- kernel/bpf/fixups.c | 41 ++++++++++++++++++++++++++----------- 3 files changed, 32 insertions(+), 15 deletions(-) diff --git a/include/linux/bpf.h b/include/linux/bpf.h index e57af902560c..8115f307444a 100644 --- a/include/linux/bpf.h +++ b/include/linux/bpf.h @@ -4165,7 +4165,7 @@ struct bpf_prog *bpf_prog_find_from_stack(void); int bpf_insn_array_init(struct bpf_map *map, const struct bpf_prog *prog); int bpf_insn_array_ready(struct bpf_map *map); void bpf_insn_array_release(struct bpf_map *map); -void bpf_insn_array_adjust(struct bpf_map *map, u32 off, u32 len); +void bpf_insn_array_adjust(struct bpf_map *map, u32 first, u32 len); void bpf_insn_array_adjust_after_remove(struct bpf_map *map, u32 off, u32 len); #ifdef CONFIG_BPF_SYSCALL diff --git a/kernel/bpf/bpf_insn_array.c b/kernel/bpf/bpf_insn_array.c index 5874abd1f743..350ecea8f1e8 100644 --- a/kernel/bpf/bpf_insn_array.c +++ b/kernel/bpf/bpf_insn_array.c @@ -235,7 +235,7 @@ void bpf_insn_array_release(struct bpf_map *map) atomic_set(&insn_array->used, 0); } -void bpf_insn_array_adjust(struct bpf_map *map, u32 off, u32 len) +void bpf_insn_array_adjust(struct bpf_map *map, u32 first, u32 len) { struct bpf_insn_array *insn_array = cast_insn_array(map); int i; @@ -244,7 +244,7 @@ void bpf_insn_array_adjust(struct bpf_map *map, u32 off, u32 len) return; for (i = 0; i < map->max_entries; i++) { - if (insn_array->values[i].xlated_off <= off) + if (insn_array->values[i].xlated_off < first) continue; if (insn_array->values[i].xlated_off == INSN_DELETED) continue; diff --git a/kernel/bpf/fixups.c b/kernel/bpf/fixups.c index aa3fb7b87fb0..e132bd0600c7 100644 --- a/kernel/bpf/fixups.c +++ b/kernel/bpf/fixups.c @@ -213,12 +213,18 @@ static int get_callee_stack_depth(struct bpf_verifier_env *env, } #endif +enum bpf_patch_mode { + BPF_APPEND, + BPF_PREPEND, +}; + /* single env->prog->insni[off] instruction was replaced with the range * insni[off, off + cnt). Adjust corresponding insn_aux_data by copying * [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, + enum bpf_patch_mode mode) { struct bpf_insn_aux_data *data = env->insn_aux_data; struct bpf_insn *insn = new_prog->insnsi; @@ -251,9 +257,10 @@ static void adjust_insn_aux_data(struct bpf_verifier_env *env, * new instructions by the above memmove and memset, but the indirect jump target is * actually the first instruction, so move it back. This also matches with the behavior * of bpf_insn_array_adjust(), which preserves xlated_off to point to the first new - * instruction. + * instruction. For BPF_PREPEND the original instruction is the last one, so the flag + * already sits where needed. */ - if (data[off + cnt - 1].indirect_target) { + if (mode == BPF_APPEND && data[off + cnt - 1].indirect_target) { data[off].indirect_target = 1; data[off + cnt - 1].indirect_target = 0; } @@ -273,7 +280,7 @@ static void adjust_subprog_starts(struct bpf_verifier_env *env, u32 off, u32 len } } -static void adjust_insn_arrays(struct bpf_verifier_env *env, u32 off, u32 len) +static void adjust_insn_arrays(struct bpf_verifier_env *env, u32 first, u32 len) { int i; @@ -281,7 +288,7 @@ static void adjust_insn_arrays(struct bpf_verifier_env *env, u32 off, u32 len) return; for (i = 0; i < env->insn_array_map_cnt; i++) - bpf_insn_array_adjust(env->insn_array_maps[i], off, len); + bpf_insn_array_adjust(env->insn_array_maps[i], first, len); } static void adjust_insn_arrays_after_remove(struct bpf_verifier_env *env, u32 off, u32 len) @@ -306,8 +313,9 @@ 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) +static struct bpf_prog *__bpf_patch_insn_data(struct bpf_verifier_env *env, u32 off, + const struct bpf_insn *patch, u32 len, + enum bpf_patch_mode mode) { struct bpf_prog *new_prog; struct bpf_insn_aux_data *new_data = NULL; @@ -331,13 +339,19 @@ 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, mode); adjust_subprog_starts(env, off, len); - adjust_insn_arrays(env, off, len); + adjust_insn_arrays(env, mode == BPF_PREPEND ? off : off + 1, len); adjust_poke_descs(new_prog, off, len); return new_prog; } +struct bpf_prog *bpf_patch_insn_data(struct bpf_verifier_env *env, u32 off, + const struct bpf_insn *patch, u32 len) +{ + return __bpf_patch_insn_data(env, off, patch, len, BPF_APPEND); +} + /* * For all jmp insns in a given 'prog' that point to 'tgt_idx' insn adjust the * jump offset by 'delta'. @@ -755,7 +769,8 @@ 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, + BPF_PREPEND); if (!new_prog) return -ENOMEM; env->prog = new_prog; @@ -778,7 +793,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, + BPF_PREPEND); if (!new_prog) return -ENOMEM; @@ -2443,7 +2459,8 @@ 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, + BPF_PREPEND); if (!new_prog) return -ENOMEM; env->prog = prog = new_prog; -- 2.43.0