From: Xu Kuohai When JIT is required by the bpf program, the kernel rejects the program on JIT failure rather than falling back to the interpreter. In this case, the subprog_starts duplication/restore and bpf prog clone are unnecessary, since they are only used to restore state for the interpreter. So remove the duplication/restore for this case. Signed-off-by: Xu Kuohai --- include/linux/filter.h | 9 ++-- kernel/bpf/core.c | 104 +++++++++++++++++++++++------------------ kernel/bpf/fixups.c | 37 +++++++-------- 3 files changed, 83 insertions(+), 67 deletions(-) diff --git a/include/linux/filter.h b/include/linux/filter.h index 5c072c1360eb..2d1aa062566c 100644 --- a/include/linux/filter.h +++ b/include/linux/filter.h @@ -1356,7 +1356,8 @@ int bpf_jit_get_func_addr(const struct bpf_prog *prog, const char *bpf_jit_get_prog_name(struct bpf_prog *prog); -struct bpf_prog *bpf_jit_blind_constants(struct bpf_verifier_env *env, struct bpf_prog *prog); +int bpf_jit_blind_constants(struct bpf_verifier_env *env, struct bpf_prog **pprog, + bool clone_needed, bool *cloned); void bpf_jit_prog_release_other(struct bpf_prog *fp, struct bpf_prog *fp_other); static inline bool bpf_prog_need_blind(const struct bpf_prog *prog) @@ -1508,9 +1509,11 @@ static inline bool bpf_prog_need_blind(const struct bpf_prog *prog) } static inline -struct bpf_prog *bpf_jit_blind_constants(struct bpf_verifier_env *env, struct bpf_prog *prog) +int bpf_jit_blind_constants(struct bpf_verifier_env *env, struct bpf_prog **pprog, + bool clone_needed, bool *cloned) { - return prog; + *cloned = false; + return 0; } static inline void bpf_jit_prog_release_other(struct bpf_prog *fp, struct bpf_prog *fp_other) diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c index 505d134cd264..ab5da3ff941a 100644 --- a/kernel/bpf/core.c +++ b/kernel/bpf/core.c @@ -1564,27 +1564,34 @@ void bpf_jit_prog_release_other(struct bpf_prog *fp, struct bpf_prog *fp_other) * Now this function is used only to blind the main prog and must be invoked only when * bpf_prog_need_blind() returns true. */ -struct bpf_prog *bpf_jit_blind_constants(struct bpf_verifier_env *env, struct bpf_prog *prog) +int bpf_jit_blind_constants(struct bpf_verifier_env *env, struct bpf_prog **pprog, + bool clone_needed, bool *cloned) { struct bpf_insn insn_buff[16], aux[2]; - struct bpf_prog *clone, *tmp; + struct bpf_prog *prog, *orig_prog, *tmp; int insn_delta, insn_cnt; struct bpf_insn *insn; int i, rewritten; - if (WARN_ON_ONCE(env && env->prog != prog)) - return ERR_PTR(-EINVAL); + *cloned = false; + if (WARN_ON_ONCE(env && env->prog != *pprog)) + return -EINVAL; - clone = bpf_prog_clone_create(prog, GFP_USER); - if (!clone) - return ERR_PTR(-ENOMEM); + prog = orig_prog = *pprog; + /* only clone the prog when we can fall back to the interpreter */ + if (clone_needed) { + prog = bpf_prog_clone_create(orig_prog, GFP_USER); + if (!prog) + return -ENOMEM; - /* make sure bpf_patch_insn_data() patches the correct prog */ - if (env) - env->prog = clone; + *cloned = true; + /* make sure bpf_patch_insn_data() patches the correct prog */ + if (env) + env->prog = prog; + } - insn_cnt = clone->len; - insn = clone->insnsi; + insn_cnt = prog->len; + insn = prog->insnsi; for (i = 0; i < insn_cnt; i++, insn++) { if (bpf_pseudo_func(insn)) { @@ -1605,42 +1612,49 @@ struct bpf_prog *bpf_jit_blind_constants(struct bpf_verifier_env *env, struct bp insn[1].code == 0) memcpy(aux, insn, sizeof(aux)); - rewritten = bpf_jit_blind_insn(insn, aux, insn_buff, - clone->aux->verifier_zext); + rewritten = bpf_jit_blind_insn(insn, aux, insn_buff, prog->aux->verifier_zext); if (!rewritten) continue; if (env) tmp = bpf_patch_insn_data(env, i, insn_buff, rewritten); else - tmp = bpf_patch_insn_single(clone, i, insn_buff, rewritten); + tmp = bpf_patch_insn_single(prog, i, insn_buff, rewritten); if (IS_ERR_OR_NULL(tmp)) { - if (env) - /* restore the original prog */ - env->prog = prog; - /* Patching may have repointed aux->prog during - * realloc from the original one, so we need to - * fix it up here on error. - */ - bpf_jit_prog_release_other(prog, clone); - return IS_ERR(tmp) ? tmp : ERR_PTR(-ENOMEM); + if (*cloned) { + /* roll back to the original prog */ + *pprog = orig_prog; + if (env) + env->prog = orig_prog; + /* Patching may have repointed aux->prog during + * realloc from the original one, so we need to + * fix it up here on error. + */ + bpf_jit_prog_release_other(orig_prog, prog); + } else { + /* just keep the latest successfully patched prog */ + *pprog = prog; + } + return IS_ERR(tmp) ? PTR_ERR(tmp) : -ENOMEM; } - clone = tmp; + prog = tmp; insn_delta = rewritten - 1; if (env) - env->prog = clone; + env->prog = prog; /* Walk new program and skip insns we just inserted. */ - insn = clone->insnsi + i + insn_delta; + insn = prog->insnsi + i + insn_delta; insn_cnt += insn_delta; i += insn_delta; } - clone->blinded = 1; - return clone; + prog->blinded = 1; + *pprog = prog; + + return 0; } bool bpf_insn_is_indirect_target(const struct bpf_verifier_env *env, const struct bpf_prog *prog, @@ -2630,33 +2644,33 @@ static bool bpf_prog_select_interpreter(struct bpf_prog *fp) return select_interpreter; } -static struct bpf_prog *bpf_prog_jit_compile(struct bpf_verifier_env *env, struct bpf_prog *prog) +static struct bpf_prog *bpf_prog_jit_compile(struct bpf_verifier_env *env, struct bpf_prog *prog, + bool jit_needed) { #ifdef CONFIG_BPF_JIT + int err; + bool cloned = false; struct bpf_prog *orig_prog; if (!bpf_prog_need_blind(prog)) return bpf_int_jit_compile(env, prog); orig_prog = prog; - prog = bpf_jit_blind_constants(env, prog); - /* - * If blinding was requested and we failed during blinding, we must fall - * back to the interpreter. - */ - if (IS_ERR(prog)) - goto out_restore; + err = bpf_jit_blind_constants(env, &prog, !jit_needed, &cloned); + if (err) + goto out; prog = bpf_int_jit_compile(env, prog); - if (prog->jited) { - bpf_jit_prog_release_other(prog, orig_prog); - return prog; + if (cloned) { + if (prog->jited) { + bpf_jit_prog_release_other(prog, orig_prog); + } else { + bpf_jit_prog_release_other(orig_prog, prog); + prog = orig_prog; + } } - bpf_jit_prog_release_other(orig_prog, prog); - -out_restore: - prog = orig_prog; +out: #endif return prog; } @@ -2686,7 +2700,7 @@ struct bpf_prog *__bpf_prog_select_runtime(struct bpf_verifier_env *env, struct if (*err) return fp; - fp = bpf_prog_jit_compile(env, fp); + fp = bpf_prog_jit_compile(env, fp, jit_needed); bpf_prog_jit_attempt_done(fp); if (!fp->jited && jit_needed) { *err = -ENOTSUPP; diff --git a/kernel/bpf/fixups.c b/kernel/bpf/fixups.c index 2cdad2395b71..6da0f8d0b761 100644 --- a/kernel/bpf/fixups.c +++ b/kernel/bpf/fixups.c @@ -996,6 +996,8 @@ static u32 *bpf_dup_subprog_starts(struct bpf_verifier_env *env) static void bpf_restore_subprog_starts(struct bpf_verifier_env *env, u32 *orig_starts) { + if (!orig_starts) + return; for (int i = 0; i < env->subprog_cnt; i++) env->subprog_info[i].start = orig_starts[i]; /* restore the start of fake 'exit' subprog as well */ @@ -1273,35 +1275,33 @@ static int jit_subprogs(struct bpf_verifier_env *env) int bpf_jit_subprogs(struct bpf_verifier_env *env) { int err, i; - bool blinded = false; struct bpf_insn *insn; struct bpf_prog *prog, *orig_prog; - u32 *orig_subprog_starts; + u32 *orig_subprog_starts = NULL; + bool cloned = false; if (env->subprog_cnt <= 1) return 0; prog = orig_prog = env->prog; if (bpf_prog_need_blind(prog)) { - orig_subprog_starts = bpf_dup_subprog_starts(env); - if (!orig_subprog_starts) { - err = -ENOMEM; - goto out_cleanup; + if (!prog->jit_required) { + orig_subprog_starts = bpf_dup_subprog_starts(env); + if (!orig_subprog_starts) { + err = -ENOMEM; + goto out_cleanup; + } } - prog = bpf_jit_blind_constants(env, prog); - if (IS_ERR(prog)) { - err = -ENOMEM; - prog = orig_prog; + err = bpf_jit_blind_constants(env, &prog, !prog->jit_required, &cloned); + if (err) goto out_restore; - } - blinded = true; } err = jit_subprogs(env); if (err) goto out_jit_err; - if (blinded) { + if (cloned) { bpf_jit_prog_release_other(prog, orig_prog); kvfree(orig_subprog_starts); } @@ -1309,13 +1309,13 @@ int bpf_jit_subprogs(struct bpf_verifier_env *env) return 0; out_jit_err: - if (blinded) { + if (cloned) { bpf_jit_prog_release_other(orig_prog, prog); /* roll back to the clean original prog */ prog = env->prog = orig_prog; goto out_restore; } else { - if (err != -EFAULT) { + if (err != -EFAULT && !prog->jit_required) { /* * We will fall back to interpreter mode when err is not -EFAULT, before * that, insn->off and insn->imm should be restored to their original @@ -1344,8 +1344,7 @@ int bpf_jit_subprogs(struct bpf_verifier_env *env) int bpf_fixup_call_args(struct bpf_verifier_env *env) { #ifndef CONFIG_BPF_JIT_ALWAYS_ON - struct bpf_prog *prog = env->prog; - struct bpf_insn *insn = prog->insnsi; + struct bpf_insn *insn; int depth; #endif int i, err = 0; @@ -1371,7 +1370,7 @@ int bpf_fixup_call_args(struct bpf_verifier_env *env) return err; } #ifndef CONFIG_BPF_JIT_ALWAYS_ON - if (prog->jit_required) { + if (env->prog->jit_required) { verbose(env, "program requires BPF JIT compiler but it is not available\n"); return -EINVAL; } @@ -1388,7 +1387,7 @@ int bpf_fixup_call_args(struct bpf_verifier_env *env) verbose(env, "tail_calls are not allowed in non-JITed programs with bpf-to-bpf calls\n"); return -EINVAL; } - for (i = 0; i < prog->len; i++, insn++) { + for (i = 0, insn = env->prog->insnsi; i < env->prog->len; i++, insn++) { if (bpf_pseudo_func(insn)) { /* When JIT fails the progs with callback calls * have to be rejected, since interpreter doesn't support them yet. -- 2.47.3