From 67e686fc73b6d11316e225aa81fb30132fb7150d Mon Sep 17 00:00:00 2001 From: Greg Kroah-Hartman Date: Mon, 26 Jul 2021 11:12:51 +0200 Subject: [PATCH] Revert "bpf: Track subprog poke descriptors correctly and fix use-after-free" This reverts commit a9f36bf3613c65cb587c70fac655c775d911409b which is commit f263a81451c12da5a342d90572e317e611846f2c upstream. It breaks the Android KABI and is not needed for Android devices at this point in time. Signed-off-by: Greg Kroah-Hartman Change-Id: Id5d1b6407f9bd5228630b9a813e9799dbc448b96 --- arch/x86/net/bpf_jit_comp.c | 3 -- include/linux/bpf.h | 1 - kernel/bpf/core.c | 8 +---- kernel/bpf/verifier.c | 60 ++++++++++++++++++++++++------------- 4 files changed, 40 insertions(+), 32 deletions(-) diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c index d5fa77256058..a11796bbb9ce 100644 --- a/arch/x86/net/bpf_jit_comp.c +++ b/arch/x86/net/bpf_jit_comp.c @@ -564,9 +564,6 @@ static void bpf_tail_call_direct_fixup(struct bpf_prog *prog) for (i = 0; i < prog->aux->size_poke_tab; i++) { poke = &prog->aux->poke_tab[i]; - if (poke->aux && poke->aux != prog->aux) - continue; - WARN_ON_ONCE(READ_ONCE(poke->tailcall_target_stable)); if (poke->reason != BPF_POKE_REASON_TAIL_CALL) diff --git a/include/linux/bpf.h b/include/linux/bpf.h index 91a74dedc92b..62ff6b56a929 100644 --- a/include/linux/bpf.h +++ b/include/linux/bpf.h @@ -760,7 +760,6 @@ struct bpf_jit_poke_descriptor { void *tailcall_target; void *tailcall_bypass; void *bypass_addr; - void *aux; union { struct { struct bpf_map *map; diff --git a/kernel/bpf/core.c b/kernel/bpf/core.c index 8b80cbe3ee1e..5e76a8a722de 100644 --- a/kernel/bpf/core.c +++ b/kernel/bpf/core.c @@ -2171,14 +2171,8 @@ static void bpf_prog_free_deferred(struct work_struct *work) #endif if (aux->dst_trampoline) bpf_trampoline_put(aux->dst_trampoline); - for (i = 0; i < aux->func_cnt; i++) { - /* We can just unlink the subprog poke descriptor table as - * it was originally linked to the main program and is also - * released along with it. - */ - aux->func[i]->aux->poke_tab = NULL; + for (i = 0; i < aux->func_cnt; i++) bpf_jit_free(aux->func[i]); - } if (aux->func_cnt) { kfree(aux->func); bpf_prog_unlock_free(aux->prog); diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c index 1f8bf2b39d50..bf6798fb2331 100644 --- a/kernel/bpf/verifier.c +++ b/kernel/bpf/verifier.c @@ -11157,19 +11157,33 @@ static int jit_subprogs(struct bpf_verifier_env *env) goto out_free; func[i]->is_func = 1; func[i]->aux->func_idx = i; - /* Below members will be freed only at prog->aux */ + /* the btf and func_info will be freed only at prog->aux */ func[i]->aux->btf = prog->aux->btf; func[i]->aux->func_info = prog->aux->func_info; - func[i]->aux->poke_tab = prog->aux->poke_tab; - func[i]->aux->size_poke_tab = prog->aux->size_poke_tab; for (j = 0; j < prog->aux->size_poke_tab; j++) { - struct bpf_jit_poke_descriptor *poke; + u32 insn_idx = prog->aux->poke_tab[j].insn_idx; + int ret; - poke = &prog->aux->poke_tab[j]; - if (poke->insn_idx < subprog_end && - poke->insn_idx >= subprog_start) - poke->aux = func[i]->aux; + if (!(insn_idx >= subprog_start && + insn_idx <= subprog_end)) + continue; + + ret = bpf_jit_add_poke_descriptor(func[i], + &prog->aux->poke_tab[j]); + if (ret < 0) { + verbose(env, "adding tail call poke descriptor failed\n"); + goto out_free; + } + + func[i]->insnsi[insn_idx - subprog_start].imm = ret + 1; + + map_ptr = func[i]->aux->poke_tab[ret].tail_call.map; + ret = map_ptr->ops->map_poke_track(map_ptr, func[i]->aux); + if (ret < 0) { + verbose(env, "tracking tail call prog failed\n"); + goto out_free; + } } /* Use bpf_prog_F_tag to indicate functions in stack traces. @@ -11199,6 +11213,18 @@ static int jit_subprogs(struct bpf_verifier_env *env) cond_resched(); } + /* Untrack main program's aux structs so that during map_poke_run() + * we will not stumble upon the unfilled poke descriptors; each + * of the main program's poke descs got distributed across subprogs + * and got tracked onto map, so we are sure that none of them will + * be missed after the operation below + */ + for (i = 0; i < prog->aux->size_poke_tab; i++) { + map_ptr = prog->aux->poke_tab[i].tail_call.map; + + map_ptr->ops->map_poke_untrack(map_ptr, prog->aux); + } + /* at this point all bpf functions were successfully JITed * now populate all bpf_calls with correct addresses and * run last pass of JIT @@ -11267,22 +11293,14 @@ static int jit_subprogs(struct bpf_verifier_env *env) bpf_prog_free_unused_jited_linfo(prog); return 0; out_free: - /* We failed JIT'ing, so at this point we need to unregister poke - * descriptors from subprogs, so that kernel is not attempting to - * patch it anymore as we're freeing the subprog JIT memory. - */ - for (i = 0; i < prog->aux->size_poke_tab; i++) { - map_ptr = prog->aux->poke_tab[i].tail_call.map; - map_ptr->ops->map_poke_untrack(map_ptr, prog->aux); - } - /* At this point we're guaranteed that poke descriptors are not - * live anymore. We can just unlink its descriptor table as it's - * released with the main prog. - */ for (i = 0; i < env->subprog_cnt; i++) { if (!func[i]) continue; - func[i]->aux->poke_tab = NULL; + + for (j = 0; j < func[i]->aux->size_poke_tab; j++) { + map_ptr = func[i]->aux->poke_tab[j].tail_call.map; + map_ptr->ops->map_poke_untrack(map_ptr, func[i]->aux); + } bpf_jit_free(func[i]); } kfree(func);