Re: [PATCH bpf v2] bpf: Fix potential UAF in bpf_netns_link_update_prog
Pu Lehui <[email protected]>
| Newsgroups | dev.linux.lists.sashiko-reviews,org.kernel.vger.bpf |
|---|---|
| Message-ID | <[email protected]> |
On 2026/7/28 10:39, [email protected] wrote: > Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider: > > Pre-existing issues: > - [Critical] Lockless, unprotected access to `link->prog` fields allows Use-After-Free and TOCTOU bugs when a link's program is updated concurrently. > -- > > --- Patch [1]: [PATCH bpf v2] bpf: Fix potential UAF in bpf_netns_link_update_prog --- > commit afa2c5a0786c29e2f3227e6c9b10cbfcd8407bc9 > Author: Pu Lehui <[email protected]> > > bpf: Fix potential UAF in bpf_netns_link_update_prog > > This commit fixes a potential use-after-free in bpf_netns_link_update_prog > by moving the checks for old_prog and new_prog->type inside the > netns_bpf_mutex critical section. It also uses guard(mutex) to simplify > lock management. > > Link: https://lore.kernel.org/bpf/[email protected] [0] > >> diff --git a/kernel/bpf/net_namespace.c b/kernel/bpf/net_namespace.c >> index 25f30f9edaefd..81006a242618d 100644 >> --- a/kernel/bpf/net_namespace.c >> +++ b/kernel/bpf/net_namespace.c >> @@ -171,33 +171,28 @@ static int bpf_netns_link_update_prog(struct bpf_link *link, >> struct net *net; >> int idx, ret; >> >> + guard(mutex)(&netns_bpf_mutex); >> + >> if (old_prog && old_prog != link->prog) >> return -EPERM; >> if (new_prog->type != link->prog->type) >> return -EINVAL; >> > [ ... ] >> old_prog = xchg(&link->prog, new_prog); >> bpf_prog_put(old_prog); > > [Severity: Critical] > This is a pre-existing issue, but can this concurrent update race with > lockless readers of link->prog in other parts of the code? The follow patch has been covered this issue. https://lore.kernel.org/bpf/[email protected] > > For example, in bpf_link_get_info_by_fd(): > > kernel/bpf/syscall.c:bpf_link_get_info_by_fd() { > ... > if (link->prog) > info.prog_id = link->prog->aux->id; > ... > } > > Or in bpf_link_show_fdinfo(): > > kernel/bpf/syscall.c:bpf_link_show_fdinfo() { > ... > const struct bpf_prog *prog = link->prog; > ... > if (prog) { > bin2hex(prog_tag, prog->tag, sizeof(prog->tag)); > ... > } > > Could these lockless reads lead to a use-after-free if a thread executing > bpf_netns_link_update_prog() swaps link->prog and drops the reference to > old_prog, and the RCU grace period elapses before the reader accesses the > freed program's fields? > > Similarly, in bpf_mprog_link() in kernel/bpf/mprog.c: > > kernel/bpf/mprog.c:bpf_mprog_link() { > ... > if (type && link->prog->type != type) { > bpf_link_put(link); > return -EINVAL; > } > > tuple->link = link; > tuple->prog = link->prog; > ... > } > > Could this lead to a time-of-check to time-of-use error or a dangling > tuple->prog pointer if a link update occurs concurrently? >