Re: [PATCH bpf v4 1/4] bpf: Fix potential UAF in bpf_netns_link_update_prog

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found 1 potential issue(s) to consider:

Pre-existing issues:
- [High] Lockless access to `link->prog` in generic BPF link info retrieval functions and mprog logic can lead to a Use-After-Free when an updateable link replaces its program concurrently.
--

--- Patch [1]: [PATCH bpf v4 1/4] bpf: Fix potential UAF in bpf_netns_link_update_prog ---
commit 1e9631860a41f1ee87e0b9afa5e2e3ca3f35147e
Author: Pu Lehui <pulehui@huawei.com>

bpf: Fix potential UAF in bpf_netns_link_update_prog
    
This commit fixes a potential use-after-free race condition in
bpf_netns_link_update_prog. By moving the old_prog and prog->type checks
inside the netns_bpf_mutex critical section, it prevents concurrent
BPF_LINK_UPDATE operations from freeing the program while it is being accessed.

> diff --git a/kernel/bpf/net_namespace.c b/kernel/bpf/net_namespace.c
> index 25f30f9edaefd..9fc62db1441c6 100644
> --- a/kernel/bpf/net_namespace.c
> +++ b/kernel/bpf/net_namespace.c
> @@ -171,13 +171,17 @@ static int bpf_netns_link_update_prog(struct bpf_link *link,
>  	struct net *net;
>  	int idx, ret;
>  
> -	if (old_prog && old_prog != link->prog)
> -		return -EPERM;
> -	if (new_prog->type != link->prog->type)
> -		return -EINVAL;
> -
>  	mutex_lock(&netns_bpf_mutex);
>  
> +	if (old_prog && old_prog != link->prog) {
> +		ret = -EPERM;
> +		goto out_unlock;
> +	}
> +	if (new_prog->type != link->prog->type) {
> +		ret = -EINVAL;
> +		goto out_unlock;
> +	}
> +

[Severity: High]
This isn't a bug introduced by this patch, but while looking at the lockless
accesses of link->prog being fixed here, I noticed other places that might
have similar race conditions.

For example, in kernel/bpf/syscall.c, bpf_link_show_fdinfo() reads link->prog
locklessly:

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));
    ...
}

Similarly, bpf_link_get_info_by_fd() in the same file accesses
link->prog->aux->id locklessly, and bpf_mprog_link() in kernel/bpf/mprog.c
checks link->prog->type without locks.

If a user-space thread calls one of these info retrieval functions and is
preempted, could a concurrent BPF_LINK_UPDATE (like tcx_link_update or
the one being fixed in this patch) swap out link->prog via xchg() and free
it?

Since the reading threads do not appear to hold rcu_read_lock() or take a
reference on the program, it seems the RCU grace period could complete,
resulting in a use-after-free when they resume and dereference prog->tag or
prog->aux->id.

Should we add RCU read locks or reference counting around these generic info
retrieval paths to prevent the program from being freed out from under them?

>  	net = net_link->net;
>  	if (!net || !check_net(net)) {
>  		/* Link auto-detached or netns dying */

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260720134547.1289964-1-pulehui@huaweicloud.com?part=1