[PATCH bpf-next v8 3/8] bpf: Introduce the bpf_list_del kfunc.

Chengkaitao posted 8 patches 3 weeks ago
There is a newer version of this series
[PATCH bpf-next v8 3/8] bpf: Introduce the bpf_list_del kfunc.
Posted by Chengkaitao 3 weeks ago
From: Kaitao Cheng <chengkaitao@kylinos.cn>

If a user holds ownership of a node in the middle of a list, they
can directly remove it from the list without strictly adhering to
deletion rules from the head or tail.

We have added an additional parameter bpf_list_head *head to
bpf_list_del, as the verifier requires the head parameter to
check whether the lock is being held.

This is typically paired with bpf_refcount. After calling
bpf_list_del, it is generally necessary to drop the reference to
the list node twice to prevent reference count leaks.

Signed-off-by: Kaitao Cheng <chengkaitao@kylinos.cn>
---
 kernel/bpf/helpers.c  | 11 +++++++++++
 kernel/bpf/verifier.c |  4 ++++
 2 files changed, 15 insertions(+)

diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
index e87b263c5fe6..dac346eb1e2f 100644
--- a/kernel/bpf/helpers.c
+++ b/kernel/bpf/helpers.c
@@ -2437,6 +2437,8 @@ static struct bpf_list_node *__bpf_list_del(struct bpf_list_head *head,
 	 */
 	if (unlikely(!h->next))
 		INIT_LIST_HEAD(h);
+
+	/* verifier to guarantee n is a list node rather than the head */
 	if (list_empty(h))
 		return NULL;
 
@@ -2463,6 +2465,14 @@ __bpf_kfunc struct bpf_list_node *bpf_list_pop_back(struct bpf_list_head *head)
 	return __bpf_list_del(head, h->prev);
 }
 
+__bpf_kfunc struct bpf_list_node *bpf_list_del(struct bpf_list_head *head,
+					       struct bpf_list_node *node)
+{
+	struct bpf_list_node_kern *kn = (void *)node;
+
+	return __bpf_list_del(head, &kn->list_head);
+}
+
 __bpf_kfunc struct bpf_list_node *bpf_list_front(struct bpf_list_head *head)
 {
 	struct list_head *h = (struct list_head *)head;
@@ -4549,6 +4559,7 @@ BTF_ID_FLAGS(func, bpf_list_push_front_impl)
 BTF_ID_FLAGS(func, bpf_list_push_back_impl)
 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)
 BTF_ID_FLAGS(func, bpf_list_front, KF_RET_NULL)
 BTF_ID_FLAGS(func, bpf_list_back, KF_RET_NULL)
 BTF_ID_FLAGS(func, bpf_task_acquire, KF_ACQUIRE | KF_RCU | KF_RET_NULL)
diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
index 64c1f8343dfa..e928ad4290c7 100644
--- a/kernel/bpf/verifier.c
+++ b/kernel/bpf/verifier.c
@@ -12508,6 +12508,7 @@ enum special_kfunc_type {
 	KF_bpf_list_push_back_impl,
 	KF_bpf_list_pop_front,
 	KF_bpf_list_pop_back,
+	KF_bpf_list_del,
 	KF_bpf_list_front,
 	KF_bpf_list_back,
 	KF_bpf_cast_to_kern_ctx,
@@ -12568,6 +12569,7 @@ BTF_ID(func, bpf_list_push_front_impl)
 BTF_ID(func, bpf_list_push_back_impl)
 BTF_ID(func, bpf_list_pop_front)
 BTF_ID(func, bpf_list_pop_back)
+BTF_ID(func, bpf_list_del)
 BTF_ID(func, bpf_list_front)
 BTF_ID(func, bpf_list_back)
 BTF_ID(func, bpf_cast_to_kern_ctx)
@@ -12644,6 +12646,7 @@ static const enum special_kfunc_type bpf_list_api_kfuncs[] = {
 	KF_bpf_list_push_back_impl,
 	KF_bpf_list_pop_front,
 	KF_bpf_list_pop_back,
+	KF_bpf_list_del,
 	KF_bpf_list_front,
 	KF_bpf_list_back,
 };
@@ -12652,6 +12655,7 @@ static const enum special_kfunc_type bpf_list_api_kfuncs[] = {
 static const enum special_kfunc_type bpf_list_node_api_kfuncs[] = {
 	KF_bpf_list_push_front_impl,
 	KF_bpf_list_push_back_impl,
+	KF_bpf_list_del,
 };
 
 /* Kfuncs that take an rbtree node argument (bpf_rb_node *). */
-- 
2.50.1 (Apple Git-155)
Re: [PATCH bpf-next v8 3/8] bpf: Introduce the bpf_list_del kfunc.
Posted by Emil Tsalapatis 2 weeks, 3 days ago
On Mon Mar 16, 2026 at 7:28 AM EDT, Chengkaitao wrote:
> From: Kaitao Cheng <chengkaitao@kylinos.cn>
>
> If a user holds ownership of a node in the middle of a list, they
> can directly remove it from the list without strictly adhering to
> deletion rules from the head or tail.
>

Not sure what you mean by this, would saying "remove any node from a
linked list" work:

> We have added an additional parameter bpf_list_head *head to
> bpf_list_del, as the verifier requires the head parameter to
> check whether the lock is being held.
>
> This is typically paired with bpf_refcount. After calling
> bpf_list_del, it is generally necessary to drop the reference to
> the list node twice to prevent reference count leaks.
>

This is pretty confusing as a convention, and it's obvious in patch
8/8. Is it possible to hide this complexity from the user?

> Signed-off-by: Kaitao Cheng <chengkaitao@kylinos.cn>
> ---
>  kernel/bpf/helpers.c  | 11 +++++++++++
>  kernel/bpf/verifier.c |  4 ++++
>  2 files changed, 15 insertions(+)
>
> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> index e87b263c5fe6..dac346eb1e2f 100644
> --- a/kernel/bpf/helpers.c
> +++ b/kernel/bpf/helpers.c
> @@ -2437,6 +2437,8 @@ static struct bpf_list_node *__bpf_list_del(struct bpf_list_head *head,
>  	 */
>  	if (unlikely(!h->next))
>  		INIT_LIST_HEAD(h);
> +
> +	/* verifier to guarantee n is a list node rather than the head */

Why add this comment here? At the very least you'd need to add this in
the previous patch, but I think it's not particularly useful in general.
It definitely doesn't work here because it's for existing code.


>  	if (list_empty(h))
>  		return NULL;
>  
> @@ -2463,6 +2465,14 @@ __bpf_kfunc struct bpf_list_node *bpf_list_pop_back(struct bpf_list_head *head)
>  	return __bpf_list_del(head, h->prev);
>  }
>  
> +__bpf_kfunc struct bpf_list_node *bpf_list_del(struct bpf_list_head *head,
> +					       struct bpf_list_node *node)
> +{
> +	struct bpf_list_node_kern *kn = (void *)node;
> +
> +	return __bpf_list_del(head, &kn->list_head);

See the Sashiko review: https://sashiko.dev/#/patchset/20260316112843.78657-1-pilgrimtao%40gmail.com

1) The WARN_ON() issue is real, you'd need to adjust the warning.
2) The more important problem is the reuse bug. You need to make sure
any elements in a list that don't get freed by during
bpf_list_head_free() have NULL'ed left/right and owner fields.

> +}
> +
>  __bpf_kfunc struct bpf_list_node *bpf_list_front(struct bpf_list_head *head)
>  {
>  	struct list_head *h = (struct list_head *)head;
> @@ -4549,6 +4559,7 @@ BTF_ID_FLAGS(func, bpf_list_push_front_impl)
>  BTF_ID_FLAGS(func, bpf_list_push_back_impl)
>  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)
>  BTF_ID_FLAGS(func, bpf_list_front, KF_RET_NULL)
>  BTF_ID_FLAGS(func, bpf_list_back, KF_RET_NULL)
>  BTF_ID_FLAGS(func, bpf_task_acquire, KF_ACQUIRE | KF_RCU | KF_RET_NULL)
> diff --git a/kernel/bpf/verifier.c b/kernel/bpf/verifier.c
> index 64c1f8343dfa..e928ad4290c7 100644
> --- a/kernel/bpf/verifier.c
> +++ b/kernel/bpf/verifier.c
> @@ -12508,6 +12508,7 @@ enum special_kfunc_type {
>  	KF_bpf_list_push_back_impl,
>  	KF_bpf_list_pop_front,
>  	KF_bpf_list_pop_back,
> +	KF_bpf_list_del,
>  	KF_bpf_list_front,
>  	KF_bpf_list_back,
>  	KF_bpf_cast_to_kern_ctx,
> @@ -12568,6 +12569,7 @@ BTF_ID(func, bpf_list_push_front_impl)
>  BTF_ID(func, bpf_list_push_back_impl)
>  BTF_ID(func, bpf_list_pop_front)
>  BTF_ID(func, bpf_list_pop_back)
> +BTF_ID(func, bpf_list_del)
>  BTF_ID(func, bpf_list_front)
>  BTF_ID(func, bpf_list_back)
>  BTF_ID(func, bpf_cast_to_kern_ctx)
> @@ -12644,6 +12646,7 @@ static const enum special_kfunc_type bpf_list_api_kfuncs[] = {
>  	KF_bpf_list_push_back_impl,
>  	KF_bpf_list_pop_front,
>  	KF_bpf_list_pop_back,
> +	KF_bpf_list_del,
>  	KF_bpf_list_front,
>  	KF_bpf_list_back,
>  };
> @@ -12652,6 +12655,7 @@ static const enum special_kfunc_type bpf_list_api_kfuncs[] = {
>  static const enum special_kfunc_type bpf_list_node_api_kfuncs[] = {
>  	KF_bpf_list_push_front_impl,
>  	KF_bpf_list_push_back_impl,
> +	KF_bpf_list_del,
>  };
>  
>  /* Kfuncs that take an rbtree node argument (bpf_rb_node *). */
Re: [PATCH bpf-next v8 3/8] bpf: Introduce the bpf_list_del kfunc.
Posted by bot+bpf-ci@kernel.org 3 weeks ago
> diff --git a/kernel/bpf/helpers.c b/kernel/bpf/helpers.c
> index e87b263c5fe6..dac346eb1e2f 100644
> --- a/kernel/bpf/helpers.c
> +++ b/kernel/bpf/helpers.c
> @@ -2437,6 +2437,8 @@ static struct bpf_list_node *__bpf_list_del(struct bpf_list_head *head,
>  	 */
>  	if (unlikely(!h->next))
>  		INIT_LIST_HEAD(h);
> +
> +	/* verifier to guarantee n is a list node rather than the head */
>  	if (list_empty(h))
>  		return NULL;

Is this comment accurate for all callers of __bpf_list_del()?

For bpf_list_pop_front() and bpf_list_pop_back(), n is derived
from h->next or h->prev. When the list is empty, h->next == h,
so n is the head itself:

kernel/bpf/helpers.c:bpf_list_pop_front() {
    struct list_head *h = (void *)head;
    return __bpf_list_del(head, h->next);
                                ^^^^^^^ h when list is empty
}

In that case, the list_empty() check is the runtime defense
against n == h, not the verifier. The verifier type guarantee
only applies to the new bpf_list_del() path where node is a
separate bpf_list_node argument.

Since __bpf_list_del() is shared by all three callers, this
comment could mislead a future reader into thinking the
list_empty() check is redundant, when it is actually essential
for the pop paths.

Would it be better to move this comment into bpf_list_del()
itself, or to qualify it as applying only to the bpf_list_del
path?

[ ... ]

---
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/23141914395