bpf.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH bpf] bpf: Fix use after free in bpf_get_prog_name
@ 2019-10-22 21:30 Daniel Borkmann
  2019-10-23  5:00 ` Alexei Starovoitov
  0 siblings, 1 reply; 2+ messages in thread
From: Daniel Borkmann @ 2019-10-22 21:30 UTC (permalink / raw)
  To: ast; +Cc: bpf, netdev, Daniel Borkmann, Yonghong Song, Martin KaFai Lau

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(-)

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)
-- 
2.21.0


^ permalink raw reply	[flat|nested] 2+ messages in thread

* Re: [PATCH bpf] bpf: Fix use after free in bpf_get_prog_name
  2019-10-22 21:30 [PATCH bpf] bpf: Fix use after free in bpf_get_prog_name Daniel Borkmann
@ 2019-10-23  5:00 ` Alexei Starovoitov
  0 siblings, 0 replies; 2+ messages in thread
From: Alexei Starovoitov @ 2019-10-23  5:00 UTC (permalink / raw)
  To: Daniel Borkmann
  Cc: Alexei Starovoitov, bpf, Network Development, Yonghong Song,
	Martin KaFai Lau

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!

^ permalink raw reply	[flat|nested] 2+ messages in thread

end of thread, other threads:[~2019-10-23  5:01 UTC | newest]

Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-10-22 21:30 [PATCH bpf] bpf: Fix use after free in bpf_get_prog_name Daniel Borkmann
2019-10-23  5:00 ` Alexei Starovoitov

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).