Skip to content

Commit f44ec87

Browse files
anakryikorostedt
authored andcommitted
bpf: put bpf_link's program when link is safe to be deallocated
In general, BPF link's underlying BPF program should be considered to be reachable through attach hook -> link -> prog chain, and, pessimistically, we have to assume that as long as link's memory is not safe to free, attach hook's code might hold a pointer to BPF program and use it. As such, it's not (generally) correct to put link's program early before waiting for RCU GPs to go through. More eager bpf_prog_put() that we currently do is mostly correct due to BPF program's release code doing similar RCU GP waiting, but as will be shown in the following patches, BPF program can be non-sleepable (and, thus, reliant on only "classic" RCU GP), while BPF link's attach hook can have sleepable semantics and needs to be protected by RCU Tasks Trace, and for such cases BPF link has to go through RCU Tasks Trace + "classic" RCU GPs before being deallocated. And so, if we put BPF program early, we might free BPF program before we free BPF link, leading to use-after-free situation. So, this patch defers bpf_prog_put() until we are ready to perform bpf_link's deallocation. At worst, this delays BPF program freeing by one extra RCU GP, but that seems completely acceptable. Alternatively, we'd need more elaborate ways to determine BPF hook, BPF link, and BPF program lifetimes, and how they relate to each other, which seems like an unnecessary complication. Note, for most BPF links we still will perform eager bpf_prog_put() and link dealloc, so for those BPF links there are no observable changes whatsoever. Only BPF links that use deferred dealloc might notice slightly delayed freeing of BPF programs. Also, to reduce code and logic duplication, extract program put + link dealloc logic into bpf_link_dealloc() helper. Link: https://lore.kernel.org/20241101181754.782341-1-andrii@kernel.org Tested-by: Jordan Rife <jrife@google.com> Signed-off-by: Andrii Nakryiko <andrii@kernel.org> Signed-off-by: Steven Rostedt (Google) <rostedt@goodmis.org>
1 parent 242b32d commit f44ec87

File tree

1 file changed

+17
-5
lines changed

1 file changed

+17
-5
lines changed

kernel/bpf/syscall.c

Lines changed: 17 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2976,12 +2976,24 @@ void bpf_link_inc(struct bpf_link *link)
29762976
atomic64_inc(&link->refcnt);
29772977
}
29782978

2979+
static void bpf_link_dealloc(struct bpf_link *link)
2980+
{
2981+
/* now that we know that bpf_link itself can't be reached, put underlying BPF program */
2982+
if (link->prog)
2983+
bpf_prog_put(link->prog);
2984+
2985+
/* free bpf_link and its containing memory */
2986+
if (link->ops->dealloc_deferred)
2987+
link->ops->dealloc_deferred(link);
2988+
else
2989+
link->ops->dealloc(link);
2990+
}
2991+
29792992
static void bpf_link_defer_dealloc_rcu_gp(struct rcu_head *rcu)
29802993
{
29812994
struct bpf_link *link = container_of(rcu, struct bpf_link, rcu);
29822995

2983-
/* free bpf_link and its containing memory */
2984-
link->ops->dealloc_deferred(link);
2996+
bpf_link_dealloc(link);
29852997
}
29862998

29872999
static void bpf_link_defer_dealloc_mult_rcu_gp(struct rcu_head *rcu)
@@ -3003,7 +3015,6 @@ static void bpf_link_free(struct bpf_link *link)
30033015
sleepable = link->prog->sleepable;
30043016
/* detach BPF program, clean up used resources */
30053017
ops->release(link);
3006-
bpf_prog_put(link->prog);
30073018
}
30083019
if (ops->dealloc_deferred) {
30093020
/* schedule BPF link deallocation; if underlying BPF program
@@ -3014,8 +3025,9 @@ static void bpf_link_free(struct bpf_link *link)
30143025
call_rcu_tasks_trace(&link->rcu, bpf_link_defer_dealloc_mult_rcu_gp);
30153026
else
30163027
call_rcu(&link->rcu, bpf_link_defer_dealloc_rcu_gp);
3017-
} else if (ops->dealloc)
3018-
ops->dealloc(link);
3028+
} else if (ops->dealloc) {
3029+
bpf_link_dealloc(link);
3030+
}
30193031
}
30203032

30213033
static void bpf_link_put_deferred(struct work_struct *work)

0 commit comments

Comments
 (0)