Re: [PATCH v1 bpf-next 02/18] bpf: Add a btf_id_set8_entry helper

[sashiko-bot@kernel.org]

Thank you for your contribution! Sashiko AI review found no new issues
in this patch, but noticed 1 related pre-existing issue it does not
introduce:
- [High] bpf: kfunc sets destruction on failure in
  btf_populate_kfunc_set() [1]
--

commit 0f99bfedbc98b8fbe7b7a25dd4038c3e714adb73
Author: Anton Protopopov <a.s.protopopov@gmail.com>

bpf: Add a btf_id_set8_entry helper

This commit names the element type of struct btf_id_set8 as struct
btf_id_set8_pair, introduces btf_id_set8_entry() to look up a pair, and
changes btf_id_set8_contains() to return a boolean.

> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> index 228b93d4f8c94..61d2df6776a99 100644
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c
> @@ -9973,8 +9973,8 @@ static u32 *btf_kfunc_id_set_contains(const struct btf *btf,
>  				      enum btf_kfunc_hook hook,
>  				      u32 kfunc_btf_id)
>  {
> +	struct btf_id_set8_pair *pair;
>  	struct btf_id_set8 *set;
> -	u32 *id;
>  
>  	if (hook >= BTF_KFUNC_HOOK_MAX)
>  		return NULL;
> @@ -9983,11 +9983,10 @@ static u32 *btf_kfunc_id_set_contains(const struct btf *btf,
>  	set = btf->kfunc_set_tab->sets[hook];
>  	if (!set)
>  		return NULL;
> -	id = btf_id_set8_contains(set, kfunc_btf_id);
> -	if (!id)
> +	pair = btf_id_set8_entry(set, kfunc_btf_id);
> +	if (!pair)
>  		return NULL;
> -	/* The flags for BTF ID are located next to it */
> -	return id + 1;
> +	return &pair->flags;
>  }

[Severity: High]
This problem wasn't introduced by this patch, but in the paired
btf_populate_kfunc_set() function that populates btf->kfunc_set_tab, does an
error on a later registration destroy all previously registered kfunc sets
[1]?

kernel/bpf/btf.c:btf_populate_kfunc_set() {
    ...
	/* Grow set */
	set = krealloc(tab->sets[hook],
		       struct_size(set, pairs, set_cnt + add_set->cnt),
		       GFP_KERNEL | __GFP_NOWARN);
	if (!set) {
		ret = -ENOMEM;
		goto end;
	}
    ...
end:
	btf_free_kfunc_set_tab(btf);
	return ret;
}

Multiple subsystems register kfunc sets into the shared vmlinux BTF.  If a
later registration fails in krealloc() or exceeds BTF_KFUNC_SET_MAX_CNT,
jumping to end calls btf_free_kfunc_set_tab(), which frees every hook's set
and clears btf->kfunc_set_tab, breaking all previously registered kfuncs.

[1] https://sashiko.dev/#/bug/linux-1a311d3c-46c9-4fa6-a26e-40badcd832d8

-- 
Sashiko AI review · https://sashiko.dev/#/patchset/20261106205523.1765622-1-a.s.protopopov@gmail.com?part=2