Re: [PATCH bpf] bpf: Skip unsettled links in link iterator

[sashiko-bot@kernel.org]

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

Pre-existing issues:
- [High] Preexisting race condition between btf_put() and btf_idr iteration causes refcount saturation and Use-After-Free.
--

commit c9cafeb12312032e3482a8253aec4ec8da20e91f
Author: Weiming Shi <bestswngs@gmail.com>

bpf: Skip unsettled links in link iterator

This commit fixes a use-after-free in the BPF link iterator by ensuring
unsettled links (with ID zero) are skipped during IDR traversal. It prevents
taking references on transient links that might be freed concurrently.

> diff --git a/kernel/bpf/syscall.c b/kernel/bpf/syscall.c
> index 853b47f813842..244a939b9d2da 100644
> --- a/kernel/bpf/syscall.c
> +++ b/kernel/bpf/syscall.c
> @@ -6042,7 +6042,10 @@ struct bpf_link *bpf_link_get_curr_or_next(u32 *id)
>  again:
>  	link = idr_get_next(&link_idr, id);
>  	if (link) {
> -		link = bpf_link_inc_not_zero(link);
> +		if (link->id)
> +			link = bpf_link_inc_not_zero(link);
> +		else
> +			link = ERR_PTR(-EAGAIN);

[Severity: High]
This isn't a bug introduced by this patch, but while looking at this fix for
bpf_link IDR iteration, is there a similar race condition in the BTF IDR
iteration?

In kernel/bpf/btf.c, btf_put() decrements the refcount outside the IDR lock:

kernel/bpf/btf.c:btf_put() {
    if (btf && refcount_dec_and_test(&btf->refcnt)) {
        ...
        /* btf_free_id(btf) is called later, acquiring the lock */

Concurrently, iterators like bpf_find_btf_id() or bpf_core_add_cands()
iterate over btf_idr holding btf_idr_lock and unconditionally increment
the refcount via btf_get() (which maps to refcount_inc()).

Could this lead to btf_get() incrementing a 0-refcount object before it is
removed from the IDR?

If the refcount is incremented from 0, it saturates the refcount and triggers
a kernel warning.

Furthermore, after the iterator drops btf_idr_lock to do a slow linear
search without holding rcu_read_lock(), btf_put() can proceed to remove
the ID and schedule the RCU callback to free the BTF memory.

Does this expose the iterator to a use-after-free when accessing the BTF
object later?

Would it be safer to use refcount_inc_not_zero() during BTF IDR traversal,
similar to how bpf_link_inc_not_zero() is used here?

>  		if (IS_ERR(link)) {
>  			(*id)++;
>  			goto again;

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20260914170206.170723-2-bestswngs@gmail.com?part=1