On 6/30/21 9:40 PM, John Fastabend wrote:
[...]
arch/x86/net/bpf_jit_comp.c | 4 ++++
include/linux/bpf.h | 1 +
kernel/bpf/core.c | 7 ++++++-
kernel/bpf/verifier.c | 39 +++++++------------------------------
4 files changed, 18 insertions(+), 33 deletions(-)
diff --git a/arch/x86/net/bpf_jit_comp.c b/arch/x86/net/bpf_jit_comp.c
index 2a2e290fa5d8..ce8dbc9310a9 100644
--- a/arch/x86/net/bpf_jit_comp.c
+++ b/arch/x86/net/bpf_jit_comp.c
@@ -576,6 +576,10 @@ 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 02b02cb29ce2..a7532cb3493a 100644
--- a/include/linux/bpf.h
+++ b/include/linux/bpf.h
@@ -777,6 +777,7 @@ 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 5e31ee9f7512..72810314c43b 100644
--- a/kernel/bpf/core.c
+++ b/kernel/bpf/core.c
@@ -2211,8 +2211,13 @@ 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++)
+ for (i = 0; i < aux->func_cnt; i++) {
+ /* poke_tab in subprogs are links to main prog and are
+ * freed above so delete link without kfree.
+ */
+ aux->func[i]->aux->poke_tab = NULL;
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 6e2ebcb0d66f..daa5a3f5e7b8 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -12109,30 +12109,17 @@ static int jit_subprogs(struct bpf_verifier_env *env)
/* 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++) {
- u32 insn_idx = prog->aux->poke_tab[j].insn_idx;
- int ret;
+ struct bpf_jit_poke_descriptor *poke;
- 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;
- }
+ poke = &prog->aux->poke_tab[j];
- 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;
- }
+ if (poke->insn_idx < subprog_end &&
+ poke->insn_idx >= subprog_start)
+ poke->aux = func[i]->aux;
}
This still has a bug which leads to a use-after-free. Check the out_free label:
out_free:
for (i = 0; i < env->subprog_cnt; i++) {
if (!func[i])
continue;
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);
Neither the map_poke_untrack() belongs in there nor bpf_jit_free() with non-NULL
aux->func[i]->aux->poke_tab (given it belongs to the main prog instead).
/* Use bpf_prog_F_tag to indicate functions in stack traces.
@@ -12163,18 +12150,6 @@ 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
