From: Yonghong Song <yonghong.song@linux.dev>
To: Alexei Starovoitov <alexei.starovoitov@gmail.com>
Cc: Kumar Kartikeya Dwivedi <memxor@gmail.com>,
bpf <bpf@vger.kernel.org>, Alexei Starovoitov <ast@kernel.org>,
Andrii Nakryiko <andrii@kernel.org>,
Daniel Borkmann <daniel@iogearbox.net>,
Kernel Team <kernel-team@fb.com>,
Martin KaFai Lau <martin.lau@kernel.org>
Subject: Re: [PATCH bpf-next v3 1/2] bpf: Warn with bpf_unreachable() kfunc maybe due to uninitialized variable
Date: Tue, 20 May 2025 11:01:16 -0700 [thread overview]
Message-ID: <1330496e-5dda-4b42-9524-4bfcfeb50ba7@linux.dev> (raw)
In-Reply-To: <CAADnVQLvN-TshyvkY3u9MYc7h_og=LWz7Ldf2k_33VRDqKsUZw@mail.gmail.com>
On 5/20/25 12:29 AM, Alexei Starovoitov wrote:
> On Tue, May 20, 2025 at 8:25 AM Yonghong Song <yonghong.song@linux.dev> wrote:
>>
>>
>> On 5/19/25 6:48 AM, Alexei Starovoitov wrote:
>>> On Mon, May 19, 2025 at 1:34 PM Yonghong Song <yonghong.song@linux.dev> wrote:
>>>> Marc Suñé (Isovalent, part of Cisco) reported an issue where an
>>>> uninitialized variable caused generating bpf prog binary code not
>>>> working as expected. The reproducer is in [1] where the flags
>>>> “-Wall -Werror” are enabled, but there is no warning as the compiler
>>>> takes advantage of uninitialized variable to do aggressive optimization.
>>>> The optimized code looks like below:
>>>>
>>>> ; {
>>>> 0: bf 16 00 00 00 00 00 00 r6 = r1
>>>> ; bpf_printk("Start");
>>>> 1: 18 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 r1 = 0x0 ll
>>>> 0000000000000008: R_BPF_64_64 .rodata
>>>> 3: b4 02 00 00 06 00 00 00 w2 = 0x6
>>>> 4: 85 00 00 00 06 00 00 00 call 0x6
>>>> ; DEFINE_FUNC_CTX_POINTER(data)
>>>> 5: 61 61 4c 00 00 00 00 00 w1 = *(u32 *)(r6 + 0x4c)
>>>> ; bpf_printk("pre ipv6_hdrlen_offset");
>>>> 6: 18 01 00 00 06 00 00 00 00 00 00 00 00 00 00 00 r1 = 0x6 ll
>>>> 0000000000000030: R_BPF_64_64 .rodata
>>>> 8: b4 02 00 00 17 00 00 00 w2 = 0x17
>>>> 9: 85 00 00 00 06 00 00 00 call 0x6
>>>> <END>
>>>>
>>>> The verifier will report the following failure:
>>>> 9: (85) call bpf_trace_printk#6
>>>> last insn is not an exit or jmp
>>>>
>>>> The above verifier log does not give a clear hint about how to fix
>>>> the problem and user may take quite some time to figure out that
>>>> the issue is due to compiler taking advantage of uninitialized variable.
>>>>
>>>> In llvm internals, uninitialized variable usage may generate
>>>> 'unreachable' IR insn and these 'unreachable' IR insns may indicate
>>>> uninitialized variable impact on code optimization. So far, llvm
>>>> BPF backend ignores 'unreachable' IR hence the above code is generated.
>>>> With clang21 patch [2], those 'unreachable' IR insn are converted
>>>> to func bpf_unreachable(). In order to maintain proper control flow
>>>> graph for bpf progs, [2] also adds an 'exit' insn after bpf_unreachable()
>>>> if bpf_unreachable() is the last insn in the function.
>>>> The new code looks like:
>>>>
>>>> ; {
>>>> 0: bf 16 00 00 00 00 00 00 r6 = r1
>>>> ; bpf_printk("Start");
>>>> 1: 18 01 00 00 00 00 00 00 00 00 00 00 00 00 00 00 r1 = 0x0 ll
>>>> 0000000000000008: R_BPF_64_64 .rodata
>>>> 3: b4 02 00 00 06 00 00 00 w2 = 0x6
>>>> 4: 85 00 00 00 06 00 00 00 call 0x6
>>>> ; DEFINE_FUNC_CTX_POINTER(data)
>>>> 5: 61 61 4c 00 00 00 00 00 w1 = *(u32 *)(r6 + 0x4c)
>>>> ; bpf_printk("pre ipv6_hdrlen_offset");
>>>> 6: 18 01 00 00 06 00 00 00 00 00 00 00 00 00 00 00 r1 = 0x6 ll
>>>> 0000000000000030: R_BPF_64_64 .rodata
>>>> 8: b4 02 00 00 17 00 00 00 w2 = 0x17
>>>> 9: 85 00 00 00 06 00 00 00 call 0x6
>>>> 10: 85 10 00 00 ff ff ff ff call -0x1
>>>> 0000000000000050: R_BPF_64_32 bpf_unreachable
>>>> 11: 95 00 00 00 00 00 00 00 exit
>>>> <END>
>>>>
>>>> In kernel, a new kfunc bpf_unreachable() is added. During insn
>>>> verification, any hit with bpf_unreachable() will result in
>>>> verification failure. The kernel is able to provide better
>>>> log message for debugging.
>>>>
>>>> With llvm patch [2] and without this patch (no bpf_unreachable()
>>>> kfunc for existing kernel), e.g., for old kernels, the verifier
>>>> outputs
>>>> 10: <invalid kfunc call>
>>>> kfunc 'bpf_unreachable' is referenced but wasn't resolved
>>>> Basically, kernel does not support bpf_unreachable() kfunc.
>>>> This still didn't give clear signals about possible reason.
>>>>
>>>> With llvm patch [2] and with this patch, the verifier outputs
>>>> 10: (85) call bpf_unreachable#74479
>>>> unexpected bpf_unreachable() due to uninitialized variable?
>>>> It gives much better hints for verification failure.
>>>>
>>>> [1] https://github.com/msune/clang_bpf/blob/main/Makefile#L3
>>>> [2] https://github.com/llvm/llvm-project/pull/131731
>>>>
>>>> Signed-off-by: Yonghong Song <yonghong.song@linux.dev>
>>>> ---
>>>> kernel/bpf/helpers.c | 5 +++++
>>>> kernel/bpf/verifier.c | 5 +++++
>>>> 2 files changed, 10 insertions(+)
>>>>
>>>> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
>>>> index c1113b74e1e2..4852c36b1c51 100644
>>>> --- a/kernel/bpf/helpers.c
>>>> +++ b/kernel/bpf/helpers.c
>>>> @@ -3273,6 +3273,10 @@ __bpf_kfunc void bpf_local_irq_restore(unsigned long *flags__irq_flag)
>>>> local_irq_restore(*flags__irq_flag);
>>>> }
>>>>
>>>> +__bpf_kfunc void bpf_unreachable(void)
>>>> +{
>>>> +}
>>>> +
>>>> __bpf_kfunc_end_defs();
>>>>
>>>> BTF_KFUNCS_START(generic_btf_ids)
>>>> @@ -3386,6 +3390,7 @@ BTF_ID_FLAGS(func, bpf_copy_from_user_dynptr, KF_SLEEPABLE)
>>>> BTF_ID_FLAGS(func, bpf_copy_from_user_str_dynptr, KF_SLEEPABLE)
>>>> BTF_ID_FLAGS(func, bpf_copy_from_user_task_dynptr, KF_SLEEPABLE | KF_TRUSTED_ARGS)
>>>> BTF_ID_FLAGS(func, bpf_copy_from_user_task_str_dynptr, KF_SLEEPABLE | KF_TRUSTED_ARGS)
>>>> +BTF_ID_FLAGS(func, bpf_unreachable)
>>>> BTF_KFUNCS_END(common_btf_ids)
>>>>
>>>> static const struct btf_kfunc_id_set common_kfunc_set = {
>>>> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
>>>> index d5807d2efc92..08013e2e1697 100644
>>>> --- a/kernel/bpf/verifier.c
>>>> +++ b/kernel/bpf/verifier.c
>>>> @@ -12105,6 +12105,7 @@ enum special_kfunc_type {
>>>> KF_bpf_res_spin_unlock,
>>>> KF_bpf_res_spin_lock_irqsave,
>>>> KF_bpf_res_spin_unlock_irqrestore,
>>>> + KF_bpf_unreachable,
>>>> };
>>>>
>>>> BTF_SET_START(special_kfunc_set)
>>>> @@ -12208,6 +12209,7 @@ BTF_ID(func, bpf_res_spin_lock)
>>>> BTF_ID(func, bpf_res_spin_unlock)
>>>> BTF_ID(func, bpf_res_spin_lock_irqsave)
>>>> BTF_ID(func, bpf_res_spin_unlock_irqrestore)
>>>> +BTF_ID(func, bpf_unreachable)
>>>>
>>>> static bool is_kfunc_ret_null(struct bpf_kfunc_call_arg_meta *meta)
>>>> {
>>>> @@ -13508,6 +13510,9 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
>>>> return err;
>>>> }
>>>> __mark_btf_func_reg_size(env, regs, BPF_REG_0, sizeof(u32));
>>>> + } else if (!insn->off && insn->imm == special_kfunc_list[KF_bpf_unreachable]) {
>>> Looks good, but let's not abuse special_kfunc_list[] for this case.
>>> special_kfunc_type supposed to be in both set[] and list[].
>>> This is not the case here.
>>> It was wrong to add KF_bpf_set_dentry_xattr, bpf_iter_css_task_new,
>>> bpf_dynptr_from_skb, and many others.
>>> Let's fix this tech debt that we accumulated.
>>>
>>> special_kfunc_type should include only kfuncs that return
>>> a pointer, so that this part is triggered:
>>>
>>> } else if (btf_type_is_ptr(t)) {
>>> ptr_type = btf_type_skip_modifiers(desc_btf, t->type,
>>> &ptr_type_id);
>>>
>>> if (meta.btf == btf_vmlinux &&
>>> btf_id_set_contains(&/special_kfunc_set, meta.func_id)) {
>>>
>>> All other kfuncs shouldn't be there. They don't need to be in
>>> the special_kfunc_set.
>>>
>>> Let's split enum special_kfunc_type into what it meant to be
>>> originally (both set and list), and move all list-only kfuncs
>>> into a new array.
>>> Let's call it kfunc_ids.
>>> Then the check in this patch will look like:
>>> insn->imm == kfunc_ids[KF_bpf_unreachable]
>> IIUC, the main goal is to remove some kfuncs from special_kfunc_set
>> since they are unnecessary.
>>
>> I think we do not need an 'enum' type for special_kfunc_set since
>> the for all kfuncs in special_kfunc_set, btf_id_set_contains()
>> is used to find corresponding btf_id. So current 'enum special_kfunc_type'
>> is only used for special_kfunc_list to find proper kfunc_id's.
>>
>> I think the following change should achieve this:
>>
>> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
>> index 08013e2e1697..2cf00b06ae66 100644
>> --- a/kernel/bpf/verifier.c
>> +++ b/kernel/bpf/verifier.c
>> @@ -12060,7 +12060,7 @@ enum kfunc_ptr_arg_type {
>> KF_ARG_PTR_TO_RES_SPIN_LOCK,
>> };
>>
>> -enum special_kfunc_type {
>> +enum special_kfunc_list_type {
>> KF_bpf_obj_new_impl,
>> KF_bpf_obj_drop_impl,
>> KF_bpf_refcount_acquire_impl,
>> @@ -12126,24 +12126,10 @@ BTF_ID(func, bpf_rbtree_first)
>> BTF_ID(func, bpf_rbtree_root)
>> BTF_ID(func, bpf_rbtree_left)
>> BTF_ID(func, bpf_rbtree_right)
>> -#ifdef CONFIG_NET
>> -BTF_ID(func, bpf_dynptr_from_skb)
>> -BTF_ID(func, bpf_dynptr_from_xdp)
>> -#endif
>> BTF_ID(func, bpf_dynptr_slice)
>> BTF_ID(func, bpf_dynptr_slice_rdwr)
>> -BTF_ID(func, bpf_dynptr_clone)
>> BTF_ID(func, bpf_percpu_obj_new_impl)
>> BTF_ID(func, bpf_percpu_obj_drop_impl)
>> -BTF_ID(func, bpf_throw)
>> -BTF_ID(func, bpf_wq_set_callback_impl)
>> -#ifdef CONFIG_CGROUPS
>> -BTF_ID(func, bpf_iter_css_task_new)
>> -#endif
>> -#ifdef CONFIG_BPF_LSM
>> -BTF_ID(func, bpf_set_dentry_xattr)
>> -BTF_ID(func, bpf_remove_dentry_xattr)
>> -#endif
>> BTF_SET_END(special_kfunc_set)
>>
>> BTF_ID_LIST(special_kfunc_list)
>>
>> I renamed 'enum special_kfunc_type' to 'enum special_kfunc_list_type'
>> implying that the enum values in special_kfunc_lit_type has
>> 1:1 relation to special_kfunc_list.
>>
>> WDYT?
> I think this is not going far enough.
> We confused ourselves with the current special_kfunc_type.
> I prefer a full split where enum special_kfunc_type
> contains only kfuncs for special_kfunc_set and _list,
> and a separate enum that covers kfuncs in a new kfunc_ids[]
Okay, I see. we should have
`enum special_kfunc_type`, special_kfunc_set and special_kfunc_list
for kfuncs which are used in btf_id_set_contains().
For all other kfuncs, we will have
`enum kfunc_ids_type` and kfunc_ids
Something like below:
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 08013e2e1697..66d0163c7ddb 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -12062,7 +12062,6 @@ enum kfunc_ptr_arg_type {
enum special_kfunc_type {
KF_bpf_obj_new_impl,
- KF_bpf_obj_drop_impl,
KF_bpf_refcount_acquire_impl,
KF_bpf_list_push_front_impl,
KF_bpf_list_push_back_impl,
@@ -12072,45 +12071,19 @@ enum special_kfunc_type {
KF_bpf_list_back,
KF_bpf_cast_to_kern_ctx,
KF_bpf_rdonly_cast,
- KF_bpf_rcu_read_lock,
- KF_bpf_rcu_read_unlock,
KF_bpf_rbtree_remove,
KF_bpf_rbtree_add_impl,
KF_bpf_rbtree_first,
KF_bpf_rbtree_root,
KF_bpf_rbtree_left,
KF_bpf_rbtree_right,
- KF_bpf_dynptr_from_skb,
- KF_bpf_dynptr_from_xdp,
KF_bpf_dynptr_slice,
KF_bpf_dynptr_slice_rdwr,
- KF_bpf_dynptr_clone,
KF_bpf_percpu_obj_new_impl,
- KF_bpf_percpu_obj_drop_impl,
- KF_bpf_throw,
- KF_bpf_wq_set_callback_impl,
- KF_bpf_preempt_disable,
- KF_bpf_preempt_enable,
- KF_bpf_iter_css_task_new,
- KF_bpf_session_cookie,
- KF_bpf_get_kmem_cache,
- KF_bpf_local_irq_save,
- KF_bpf_local_irq_restore,
- KF_bpf_iter_num_new,
- KF_bpf_iter_num_next,
- KF_bpf_iter_num_destroy,
- KF_bpf_set_dentry_xattr,
- KF_bpf_remove_dentry_xattr,
- KF_bpf_res_spin_lock,
- KF_bpf_res_spin_unlock,
- KF_bpf_res_spin_lock_irqsave,
- KF_bpf_res_spin_unlock_irqrestore,
- KF_bpf_unreachable,
};
BTF_SET_START(special_kfunc_set)
BTF_ID(func, bpf_obj_new_impl)
-BTF_ID(func, bpf_obj_drop_impl)
BTF_ID(func, bpf_refcount_acquire_impl)
BTF_ID(func, bpf_list_push_front_impl)
BTF_ID(func, bpf_list_push_back_impl)
@@ -12126,29 +12099,13 @@ BTF_ID(func, bpf_rbtree_first)
BTF_ID(func, bpf_rbtree_root)
BTF_ID(func, bpf_rbtree_left)
BTF_ID(func, bpf_rbtree_right)
-#ifdef CONFIG_NET
-BTF_ID(func, bpf_dynptr_from_skb)
-BTF_ID(func, bpf_dynptr_from_xdp)
-#endif
BTF_ID(func, bpf_dynptr_slice)
BTF_ID(func, bpf_dynptr_slice_rdwr)
-BTF_ID(func, bpf_dynptr_clone)
BTF_ID(func, bpf_percpu_obj_new_impl)
-BTF_ID(func, bpf_percpu_obj_drop_impl)
-BTF_ID(func, bpf_throw)
-BTF_ID(func, bpf_wq_set_callback_impl)
-#ifdef CONFIG_CGROUPS
-BTF_ID(func, bpf_iter_css_task_new)
-#endif
-#ifdef CONFIG_BPF_LSM
-BTF_ID(func, bpf_set_dentry_xattr)
-BTF_ID(func, bpf_remove_dentry_xattr)
-#endif
BTF_SET_END(special_kfunc_set)
BTF_ID_LIST(special_kfunc_list)
BTF_ID(func, bpf_obj_new_impl)
-BTF_ID(func, bpf_obj_drop_impl)
BTF_ID(func, bpf_refcount_acquire_impl)
BTF_ID(func, bpf_list_push_front_impl)
BTF_ID(func, bpf_list_push_back_impl)
@@ -12158,14 +12115,49 @@ BTF_ID(func, bpf_list_front)
BTF_ID(func, bpf_list_back)
BTF_ID(func, bpf_cast_to_kern_ctx)
BTF_ID(func, bpf_rdonly_cast)
-BTF_ID(func, bpf_rcu_read_lock)
-BTF_ID(func, bpf_rcu_read_unlock)
BTF_ID(func, bpf_rbtree_remove)
BTF_ID(func, bpf_rbtree_add_impl)
BTF_ID(func, bpf_rbtree_first)
BTF_ID(func, bpf_rbtree_root)
BTF_ID(func, bpf_rbtree_left)
BTF_ID(func, bpf_rbtree_right)
+BTF_ID(func, bpf_dynptr_slice)
+BTF_ID(func, bpf_dynptr_slice_rdwr)
+BTF_ID(func, bpf_percpu_obj_new_impl)
+
+enum kfunc_ids_type {
+ KF_bpf_obj_drop_impl,
+ KF_bpf_rcu_read_lock,
+ KF_bpf_rcu_read_unlock,
+ KF_bpf_dynptr_from_skb,
+ KF_bpf_dynptr_from_xdp,
+ KF_bpf_dynptr_clone,
+ KF_bpf_percpu_obj_drop_impl,
+ KF_bpf_throw,
+ KF_bpf_wq_set_callback_impl,
+ KF_bpf_preempt_disable,
+ KF_bpf_preempt_enable,
+ KF_bpf_iter_css_task_new,
+ KF_bpf_session_cookie,
+ KF_bpf_get_kmem_cache,
+ KF_bpf_local_irq_save,
+ KF_bpf_local_irq_restore,
+ KF_bpf_iter_num_new,
+ KF_bpf_iter_num_next,
+ KF_bpf_iter_num_destroy,
+ KF_bpf_set_dentry_xattr,
+ KF_bpf_remove_dentry_xattr,
+ KF_bpf_res_spin_lock,
+ KF_bpf_res_spin_unlock,
+ KF_bpf_res_spin_lock_irqsave,
+ KF_bpf_res_spin_unlock_irqrestore,
+ KF_bpf_unreachable,
+};
+
+BTF_ID_LIST(kfunc_ids)
+BTF_ID(func, bpf_obj_drop_impl)
+BTF_ID(func, bpf_rcu_read_lock)
+BTF_ID(func, bpf_rcu_read_unlock)
#ifdef CONFIG_NET
BTF_ID(func, bpf_dynptr_from_skb)
BTF_ID(func, bpf_dynptr_from_xdp)
@@ -12173,10 +12165,7 @@ BTF_ID(func, bpf_dynptr_from_xdp)
BTF_ID_UNUSED
BTF_ID_UNUSED
#endif
-BTF_ID(func, bpf_dynptr_slice)
-BTF_ID(func, bpf_dynptr_slice_rdwr)
BTF_ID(func, bpf_dynptr_clone)
-BTF_ID(func, bpf_percpu_obj_new_impl)
BTF_ID(func, bpf_percpu_obj_drop_impl)
BTF_ID(func, bpf_throw)
BTF_ID(func, bpf_wq_set_callback_impl)
@@ -12223,22 +12212,22 @@ static bool is_kfunc_ret_null(struct bpf_kfunc_call_arg_meta *meta)
static bool is_kfunc_bpf_rcu_read_lock(struct bpf_kfunc_call_arg_meta *meta)
{
- return meta->func_id == special_kfunc_list[KF_bpf_rcu_read_lock];
+ return meta->func_id == kfunc_ids[KF_bpf_rcu_read_lock];
}
static bool is_kfunc_bpf_rcu_read_unlock(struct bpf_kfunc_call_arg_meta *meta)
{
- return meta->func_id == special_kfunc_list[KF_bpf_rcu_read_unlock];
+ return meta->func_id == kfunc_ids[KF_bpf_rcu_read_unlock];
}
...
@@ -21470,13 +21459,13 @@ static int fixup_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
insn_buf[2] = addr[1];
insn_buf[3] = *insn;
*cnt = 4;
- } else if (desc->func_id == special_kfunc_list[KF_bpf_obj_drop_impl] ||
- desc->func_id == special_kfunc_list[KF_bpf_percpu_obj_drop_impl] ||
+ } else if (desc->func_id == kfunc_ids[KF_bpf_obj_drop_impl] ||
+ desc->func_id == kfunc_ids[KF_bpf_percpu_obj_drop_impl] ||
desc->func_id == special_kfunc_list[KF_bpf_refcount_acquire_impl]) {
struct btf_struct_meta *kptr_struct_meta = env->insn_aux_data[insn_idx].kptr_struct_meta;
struct bpf_insn addr[2] = { BPF_LD_IMM64(BPF_REG_2, (long)kptr_struct_meta) };
- if (desc->func_id == special_kfunc_list[KF_bpf_percpu_obj_drop_impl] && kptr_struct_meta) {
+ if (desc->func_id == kfunc_ids[KF_bpf_percpu_obj_drop_impl] && kptr_struct_meta) {
verbose(env, "verifier internal error: NULL kptr_struct_meta expected at insn_idx %d\n",
insn_idx);
return -EFAULT;
So we have clear separation between special_kfunc (using btf_id_set_contains())
and list-only kfunc_ids.
>
>>> Digging through the code it looks like we made a bit of a mess there.
>>> Like this part:
>>> } else if (btf_type_is_void(t)) {
>>> if (meta.btf == btf_vmlinux &&
>>> btf_id_set_contains(&special_kfunc_set, meta.func_id)) {
>>> if (meta.func_id ==
>>> special_kfunc_list[KF_bpf_obj_drop_impl] ||
>>> meta.func_id ==
>>> special_kfunc_list[KF_bpf_percpu_obj_drop_impl]) {
>>>
>>>
>>> *obj_drop don't need to be in a set,
>>> and btf_id_set_contains() doesn't need to be called.
>>> Both kfuncs should be moved to new kfunc_ids[]
>> As you mentioned, for this one, we can do
>>
>> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
>> index 2cf00b06ae66..a3ff57eaa5f4 100644
>> --- a/kernel/bpf/verifier.c
>> +++ b/kernel/bpf/verifier.c
>> @@ -12110,7 +12110,6 @@ enum special_kfunc_list_type {
>>
>> BTF_SET_START(special_kfunc_set)
>> BTF_ID(func, bpf_obj_new_impl)
>> -BTF_ID(func, bpf_obj_drop_impl)
>> BTF_ID(func, bpf_refcount_acquire_impl)
>> BTF_ID(func, bpf_list_push_front_impl)
>> BTF_ID(func, bpf_list_push_back_impl)
>> @@ -12129,7 +12128,6 @@ BTF_ID(func, bpf_rbtree_right)
>> BTF_ID(func, bpf_dynptr_slice)
>> BTF_ID(func, bpf_dynptr_slice_rdwr)
>> BTF_ID(func, bpf_percpu_obj_new_impl)
>> -BTF_ID(func, bpf_percpu_obj_drop_impl)
>> BTF_SET_END(special_kfunc_set)
>>
>> BTF_ID_LIST(special_kfunc_list)
>> @@ -13909,7 +13907,7 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
>> if (reg_may_point_to_spin_lock(®s[BPF_REG_0]) && !regs[BPF_REG_0].id)
>> regs[BPF_REG_0].id = ++env->id_gen;
>> } else if (btf_type_is_void(t)) {
>> - if (meta.btf == btf_vmlinux && btf_id_set_contains(&special_kfunc_set, meta.func_id)) {
>> + if (meta.btf == btf_vmlinux) {
>> if (meta.func_id == special_kfunc_list[KF_bpf_obj_drop_impl] ||
>> meta.func_id == special_kfunc_list[KF_bpf_percpu_obj_drop_impl]) {
>> insn_aux->kptr_struct_meta =
> Yes. Something like this but with new enum and new kfunc_ids[], like:
> if (meta.func_id == kfunc_ids[KF_bpf_obj_drop_impl] ..
Right. kfunc_ids is used here.
>
> There is a concern that two KF_* enums may be confusing,
> since it's not obvious whether
> special_kfunc_list[KF_foo] or kfunc_ids[KF_foo] should be used.
> Need to think about how to resolve the ambiguity...
Based on current verifier logic. special_kfunc_list[KF_foo] should
be used if KF_foo is involved with btf_id_set_contains(...) checking.
>
> We also have these things for btf_ids of types:
> extern u32 btf_tracing_ids[];
> extern u32 bpf_cgroup_btf_id[];
> extern u32 bpf_local_storage_map_btf_id[];
> extern u32 btf_bpf_map_id[];
> extern u32 bpf_kmem_cache_btf_id[];
next prev parent reply other threads:[~2025-05-20 18:01 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2025-05-19 20:33 [PATCH bpf-next v3 0/2] bpf: Warn with bpf_unreachable() kfunc maybe due to uninitialized variable Yonghong Song
2025-05-19 20:33 ` [PATCH bpf-next v3 1/2] " Yonghong Song
2025-05-19 22:48 ` Alexei Starovoitov
2025-05-20 15:25 ` Yonghong Song
2025-05-20 16:29 ` Alexei Starovoitov
2025-05-20 18:01 ` Yonghong Song [this message]
2025-05-20 18:39 ` Alexei Starovoitov
2025-05-20 18:46 ` Alexei Starovoitov
2025-05-20 19:50 ` Yonghong Song
2025-05-20 19:40 ` Yonghong Song
2025-05-20 20:59 ` Yonghong Song
2025-05-20 21:14 ` Alexei Starovoitov
2025-05-19 20:33 ` [PATCH bpf-next v3 2/2] selftests/bpf: Add unit tests with bpf_unreachable() kfunc Yonghong Song
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=1330496e-5dda-4b42-9524-4bfcfeb50ba7@linux.dev \
--to=yonghong.song@linux.dev \
--cc=alexei.starovoitov@gmail.com \
--cc=andrii@kernel.org \
--cc=ast@kernel.org \
--cc=bpf@vger.kernel.org \
--cc=daniel@iogearbox.net \
--cc=kernel-team@fb.com \
--cc=martin.lau@kernel.org \
--cc=memxor@gmail.com \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox