| Message ID | 875f2906a7c1a0691f2d567b4d8e4ea2739b1e88.1571779205.git.daniel@iogearbox.net |
|---|---|
| State | Accepted |
| Delegated to: | BPF Maintainers |
| Headers | show |
| Series | [bpf] bpf: Fix use after free in bpf_get_prog_name | expand |
On Tue, Oct 22, 2019 at 2:30 PM Daniel Borkmann <daniel@iogearbox.net> wrote: > > There is one more problematic case I noticed while recently fixing BPF kallsyms > handling in cd7455f1013e ("bpf: Fix use after free in subprog's jited symbol > removal") and that is bpf_get_prog_name(). > > If BTF has been attached to the prog, then we may be able to fetch the function > signature type id in kallsyms through prog->aux->func_info[prog->aux->func_idx].type_id. > However, while the BTF object itself is torn down via RCU callback, the prog's > aux->func_info is immediately freed via kvfree(prog->aux->func_info) once the > prog's refcount either hit zero or when subprograms were already exposed via > kallsyms and we hit the error path added in 5482e9a93c83 ("bpf: Fix memleak in > aux->func_info and aux->btf"). > > This violates RCU as well since kallsyms could be walked in parallel where we > could access aux->func_info. Hence, defer kvfree() to after RCU grace period. > Looking at ba64e7d85252 ("bpf: btf: support proper non-jit func info") there > is no reason/dependency where we couldn't defer the kvfree(aux->func_info) into > the RCU callback. > > Fixes: 5482e9a93c83 ("bpf: Fix memleak in aux->func_info and aux->btf") > Fixes: ba64e7d85252 ("bpf: btf: support proper non-jit func info") > Signed-off-by: Daniel Borkmann <daniel@iogearbox.net> > Cc: Yonghong Song <yhs@fb.com> > Cc: Martin KaFai Lau <kafai@fb.com> Applied. Thanks!
diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c index bcfc362de4f2..0937719b87e2 100644 --- a/kernel/bpf/syscall.c +++ b/kernel/bpf/syscall.c @@ -1326,6 +1326,7 @@ static void __bpf_prog_put_rcu(struct rcu_head *rcu) { struct bpf_prog_aux *aux = container_of(rcu, struct bpf_prog_aux, rcu); + kvfree(aux->func_info); free_used_maps(aux); bpf_prog_uncharge_memlock(aux->prog); security_bpf_prog_free(aux); @@ -1336,7 +1337,6 @@ static void __bpf_prog_put_noref(struct bpf_prog *prog, bool deferred) { bpf_prog_kallsyms_del_all(prog); btf_put(prog->aux->btf); - kvfree(prog->aux->func_info); bpf_prog_free_linfo(prog); if (deferred)
There is one more problematic case I noticed while recently fixing BPF kallsyms handling in cd7455f1013e ("bpf: Fix use after free in subprog's jited symbol removal") and that is bpf_get_prog_name(). If BTF has been attached to the prog, then we may be able to fetch the function signature type id in kallsyms through prog->aux->func_info[prog->aux->func_idx].type_id. However, while the BTF object itself is torn down via RCU callback, the prog's aux->func_info is immediately freed via kvfree(prog->aux->func_info) once the prog's refcount either hit zero or when subprograms were already exposed via kallsyms and we hit the error path added in 5482e9a93c83 ("bpf: Fix memleak in aux->func_info and aux->btf"). This violates RCU as well since kallsyms could be walked in parallel where we could access aux->func_info. Hence, defer kvfree() to after RCU grace period. Looking at ba64e7d85252 ("bpf: btf: support proper non-jit func info") there is no reason/dependency where we couldn't defer the kvfree(aux->func_info) into the RCU callback. Fixes: 5482e9a93c83 ("bpf: Fix memleak in aux->func_info and aux->btf") Fixes: ba64e7d85252 ("bpf: btf: support proper non-jit func info") Signed-off-by: Daniel Borkmann <daniel@iogearbox.net> Cc: Yonghong Song <yhs@fb.com> Cc: Martin KaFai Lau <kafai@fb.com> --- kernel/bpf/syscall.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)