Re: [PATCH bpf v3 1/3] bpf: Fix potential UAF when reading bpf link info

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] bpf_netns_link_update_prog checks link->prog locklessly, allowing a TOCTOU race that violates atomic replacement guarantees.
- [High] Concurrent BPF_LINK_UPDATE can race with bpf_mprog_link to cause a Use-After-Free when dereferencing link->prog->type.
--

--- Patch [1]: [PATCH bpf v3 1/3] bpf: Fix potential UAF when reading bpf link info ---
commit 5b8e0ac8575ebc387f2ddc7dee211697bb704588
Author: Pu Lehui <pulehui@huawei.com>

bpf: Fix potential UAF when reading bpf link info

This commit addresses a potential Use-After-Free issue when accessing
link->prog without locks in bpf_link_show_fdinfo and bpf_link_get_info_by_fd.
It resolves the problem by acquiring both normal RCU and tasks trace RCU
read locks before dereferencing the program, preventing concurrent freeing
during bpf_link_update operations.

> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index 6db306d23b479..2458c68146b95 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -3490,13 +3491,23 @@ static void bpf_link_show_fdinfo(struct seq_file *m, struct file *filp)
>  	}
>  	seq_printf(m, "link_id:\t%u\n", link->id);
>  
> +	/* prog can be sleepable */
> +	rcu_read_lock_trace();
> +	rcu_read_lock();
> +	prog = READ_ONCE(link->prog);

[Severity: High]
This isn't a bug introduced by this patch, but since you are fixing lockless 
dereferences of link->prog here, could a similar TOCTOU race happen in 
bpf_netns_link_update_prog()?

kernel/bpf/net_namespace.c:bpf_netns_link_update_prog() {
    ...
	if (old_prog && old_prog != link->prog)
		return -EPERM;
	if (new_prog->type != link->prog->type)
		return -EINVAL;

	mutex_lock(&netns_bpf_mutex);
    ...
}

Two threads concurrently calling BPF_LINK_UPDATE on a netns link could both 
evaluate the old_prog != link->prog check locklessly and pass it. If they 
acquire netns_bpf_mutex one after the other, would the second thread 
blindly overwrite link->prog, overriding the first thread's update and 
violating the BPF_F_REPLACE API contract?

[Severity: High]
This is also a pre-existing issue, but does a similar Use-After-Free race 
exist in bpf_mprog_link()?

kernel/bpf/mprog.c:bpf_mprog_link() {
    ...
	if (type && link->prog->type != type) {
		bpf_link_put(link);
		return -EINVAL;
	}
    ...
}

If a thread calls BPF_PROG_ATTACH on a tcx hook (holding the sleepable 
rtnl_lock) and supplies a relative_fd pointing to a netns link, 
bpf_mprog_link() executes without an RCU read-side critical section.

Could this thread be preempted, allowing a concurrent BPF_LINK_UPDATE on 
the same netns link to replace and free link->prog? When the first thread 
resumes, it would dereference link->prog->type where link->prog now points 
to freed memory.

>  	if (prog) {
>  		bin2hex(prog_tag, prog->tag, sizeof(prog->tag));
> +		prog_id = prog->aux->id;
> +	}
> +	rcu_read_unlock();
> +	rcu_read_unlock_trace();

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