All of lore.kernel.org
 help / color / mirror / Atom feed
From: Kaitao Cheng <kaitao.cheng@linux.dev>
To: bot+bpf-ci@kernel.org
Cc: bpf@vger.kernel.org, ast@kernel.org, corbet@lwn.net,
	martin.lau@linux.dev, daniel@iogearbox.net, andrii@kernel.org,
	eddyz87@gmail.com, song@kernel.org, yonghong.song@linux.dev,
	john.fastabend@gmail.com, kpsingh@kernel.org, sdf@fomichev.me,
	haoluo@google.com, jolsa@kernel.org, shuah@kernel.org,
	chengkaitao@kylinos.cn, skhan@linuxfoundation.org,
	memxor@gmail.com, linux-kernel@vger.kernel.org,
	linux-doc@vger.kernel.org, vmalik@redhat.com,
	linux-kselftest@vger.kernel.org, martin.lau@kernel.org,
	clm@meta.com, ihor.solodrai@linux.dev
Subject: Re: [PATCH bpf-next v11 6/8] bpf: Add bpf_list_add to insert node after a given list node
Date: Thu, 21 May 2026 15:35:52 +0800	[thread overview]
Message-ID: <db0be5c8-13cb-4007-be06-bee3e1cbafbf@linux.dev> (raw)
In-Reply-To: <61871735d9151729b3092d42209dad08b1e38fa930bc1c9e452645240efbc40e@mail.kernel.org>

在 2026/5/21 12:08, bot+bpf-ci@kernel.org 写道:
>> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
>> index 1c69476c8a09..89579165ef4d 100644
>> --- a/kernel/bpf/helpers.c
>> +++ b/kernel/bpf/helpers.c
>> @@ -2577,6 +2577,16 @@ __bpf_kfunc int bpf_list_push_back_impl(struct bpf_list_head *head,
>>  	return bpf_list_push_back(head, node, meta__ign, off);
>>  }
>>
>> +__bpf_kfunc int bpf_list_add(struct bpf_list_head *head, struct bpf_list_node *new,
>> +			     struct bpf_list_node *prev__nonown_allowed,
>> +			     struct btf_struct_meta *meta, u64 off)
>> +{
>> +	struct bpf_list_node_kern *n = (void *)new, *p = (void *)prev__nonown_allowed;
>> +	struct list_head *prev_ptr = &p->list_head;
>> +
>> +	return __bpf_list_add(n, head, &prev_ptr, meta ? meta->record : NULL, off);
>> +}
> 
> If __bpf_list_add() fails at runtime (e.g., because prev is not in the
> list indicated by head), it drops the 'new' node via __bpf_obj_drop_impl
> in the fail path.
> 
> However, the verifier unconditionally converts 'new' into a non-owning
> reference during check_kfunc_call(). If __bpf_list_add() fails, the BPF
> program retains this non-owning reference to a freed node.
> 
> Could this allow the program to trigger a use-after-free within the lock
> critical section?
> 
> (sashiko-bot@kernel.org, v10 RESEND)
> https://lore.kernel.org/bpf/20260513204420.2BB22C19425@smtp.kernel.org/

This should be an acceptable case within the current design framework.

>>  static struct bpf_list_node *__bpf_list_del(struct bpf_list_head *head,
>>  					    struct list_head *n)
> 
> [ ... ]
> 
>> @@ -4756,6 +4766,7 @@ BTF_ID_FLAGS(func, bpf_list_push_front, KF_IMPLICIT_ARGS)
>>  BTF_ID_FLAGS(func, bpf_list_push_front_impl)
>>  BTF_ID_FLAGS(func, bpf_list_push_back, KF_IMPLICIT_ARGS)
>>  BTF_ID_FLAGS(func, bpf_list_push_back_impl)
>> +BTF_ID_FLAGS(func, bpf_list_add, KF_IMPLICIT_ARGS)
> 
> Since bpf_list_add is marked with KF_IMPLICIT_ARGS, won't the verifier
> look for a bpf_list_add_impl counterpart during program load?
> 
> Without defining and registering a bpf_list_add_impl function,
> find_kfunc_impl_proto() will fail to find the required _impl suffix in
> the vmlinux BTF.
> 
> Will this cause the verifier to unconditionally reject the kfunc?
> 
> (sashiko-bot@kernel.org, v10 RESEND)
> https://lore.kernel.org/bpf/20260513204420.2BB22C19425@smtp.kernel.org/

The target prototype with the _impl suffix has already been deprecated.

>>  BTF_ID_FLAGS(func, bpf_list_pop_front, KF_ACQUIRE | KF_RET_NULL)
>>  BTF_ID_FLAGS(func, bpf_list_pop_back, KF_ACQUIRE | KF_RET_NULL)
>>  BTF_ID_FLAGS(func, bpf_list_del, KF_ACQUIRE | KF_RET_NULL)
> 
> [ ... ]
> 
>> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
>> index 35eebb5e7769..662ad7312697 100644
>> --- a/kernel/bpf/verifier.c
>> +++ b/kernel/bpf/verifier.c
> 
> [ ... ]
> 
>> @@ -19524,8 +19527,11 @@ int bpf_fixup_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
>>  		int struct_meta_reg = BPF_REG_3;
>>  		int node_offset_reg = BPF_REG_4;
>>
>> -		/* rbtree_add has extra 'less' arg, so args-to-fixup are in diff regs */
>> -		if (is_bpf_rbtree_add_kfunc(desc->func_id)) {
>> +		/* list_add/rbtree_add have an extra arg (prev/less),
>> +		 * so args-to-fixup are in diff regs.
>> +		 */
> 
> This isn't a bug, but the multi-line comment format doesn't follow the
> BPF subsystem style. The preferred form in kernel/bpf/ has the opening
> /* on its own line.
> 
> Additionally, consider adding an is_bpf_list_add_kfunc() helper instead
> of direct comparison with special_kfunc_list[KF_bpf_list_add].
> 
> (bot+bpf-ci@kernel.org and Eduard Zingerman, v10 RESEND)
> https://lore.kernel.org/bpf/f573f7e0a5140fe6254d2ddfd3f69add455fa25c67c020c0312e2c2ceeca7c69@mail.kernel.org/

I provided a more detailed explanation in the link below.
https://lore.kernel.org/all/9e300669-8793-41a2-84a1-98fb8195e263@linux.dev/

>> +		if (desc->func_id == special_kfunc_list[KF_bpf_list_add] ||
>> +		    is_bpf_rbtree_add_kfunc(desc->func_id)) {
>>  			struct_meta_reg = BPF_REG_4;
>>  			node_offset_reg = BPF_REG_5;
>>  		}
> 
> 
> ---
> AI reviewed your patch. Please fix the bug or email reply why it's not a bug.
> See: https://github.com/kernel-patches/vmtest/blob/master/ci/claude/README.md
> 
> CI run summary: https://github.com/kernel-patches/bpf/actions/runs/26204125015

-- 
Thanks
Kaitao Cheng


  reply	other threads:[~2026-05-21  7:37 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2026-05-21  3:22 [PATCH bpf-next v11 0/8] bpf: Extend the bpf_list family of APIs Kaitao Cheng
2026-05-21  3:22 ` [PATCH bpf-next v11 1/8] bpf: refactor __bpf_list_del to take list node pointer Kaitao Cheng
2026-05-21  3:23 ` [PATCH bpf-next v11 2/8] bpf: clear list node owner and unlink before drop Kaitao Cheng
2026-05-21  4:08   ` bot+bpf-ci
2026-05-21  4:30   ` sashiko-bot
2026-05-21  6:11     ` Kaitao Cheng
2026-05-21  3:23 ` [PATCH bpf-next v11 3/8] bpf: allow non-owning list-node args via __nonown_allowed Kaitao Cheng
2026-05-21  4:08   ` bot+bpf-ci
2026-05-21  6:29     ` Kaitao Cheng
2026-05-21  3:23 ` [PATCH bpf-next v11 4/8] bpf: Introduce the bpf_list_del kfunc Kaitao Cheng
2026-05-21  4:08   ` bot+bpf-ci
2026-05-21  6:59     ` Kaitao Cheng
2026-05-21  3:23 ` [PATCH bpf-next v11 5/8] bpf: refactor __bpf_list_add to take insertion point via **prev_ptr Kaitao Cheng
2026-05-21  3:23 ` [PATCH bpf-next v11 6/8] bpf: Add bpf_list_add to insert node after a given list node Kaitao Cheng
2026-05-21  4:08   ` bot+bpf-ci
2026-05-21  7:35     ` Kaitao Cheng [this message]
2026-05-21 12:49   ` sashiko-bot
2026-05-21  3:23 ` [PATCH bpf-next v11 7/8] bpf: add bpf_list_is_first/last/empty kfuncs Kaitao Cheng
2026-05-21 13:29   ` sashiko-bot
2026-05-21  3:23 ` [PATCH bpf-next v11 8/8] selftests/bpf: Add test cases for bpf_list_del/add/is_first/is_last/empty Kaitao Cheng
2026-05-21  4:08   ` bot+bpf-ci
2026-05-21 10:00 ` [PATCH bpf-next v11 0/8] bpf: Extend the bpf_list family of APIs patchwork-bot+netdevbpf

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=db0be5c8-13cb-4007-be06-bee3e1cbafbf@linux.dev \
    --to=kaitao.cheng@linux.dev \
    --cc=andrii@kernel.org \
    --cc=ast@kernel.org \
    --cc=bot+bpf-ci@kernel.org \
    --cc=bpf@vger.kernel.org \
    --cc=chengkaitao@kylinos.cn \
    --cc=clm@meta.com \
    --cc=corbet@lwn.net \
    --cc=daniel@iogearbox.net \
    --cc=eddyz87@gmail.com \
    --cc=haoluo@google.com \
    --cc=ihor.solodrai@linux.dev \
    --cc=john.fastabend@gmail.com \
    --cc=jolsa@kernel.org \
    --cc=kpsingh@kernel.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-kselftest@vger.kernel.org \
    --cc=martin.lau@kernel.org \
    --cc=martin.lau@linux.dev \
    --cc=memxor@gmail.com \
    --cc=sdf@fomichev.me \
    --cc=shuah@kernel.org \
    --cc=skhan@linuxfoundation.org \
    --cc=song@kernel.org \
    --cc=vmalik@redhat.com \
    --cc=yonghong.song@linux.dev \
    /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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.