Chengkaitao <[email protected]> writes:
> From: Kaitao Cheng <[email protected]>
>
> Add a node to both an rbtree and a list, retrieve the node from
> the rbtree, use the obtained node pointer to remove it from the
> list, and finally free the node.
>
> To verify the validity of bpf_list_del, also expect the verifier
> to reject calls to bpf_list_del made without holding the spin_lock.
>
> Signed-off-by: Kaitao Cheng <[email protected]>
> ---
> .../testing/selftests/bpf/bpf_experimental.h | 11 +++
> .../selftests/bpf/progs/refcounted_kptr.c | 71 +++++++++++++++++++
> 2 files changed, 82 insertions(+)
>
> diff --git a/tools/testing/selftests/bpf/bpf_experimental.h
> b/tools/testing/selftests/bpf/bpf_experimental.h
> index 4b7210c318dd..54ec9d307fdc 100644
> --- a/tools/testing/selftests/bpf/bpf_experimental.h
> +++ b/tools/testing/selftests/bpf/bpf_experimental.h
> @@ -99,6 +99,17 @@ extern struct bpf_list_node *bpf_list_pop_front(struct
> bpf_list_head *head) __ks
> */
> extern struct bpf_list_node *bpf_list_pop_back(struct bpf_list_head *head)
> __ksym;
>
> +/* Description
> + * Remove 'node' from the BPF linked list with head 'head'.
> + * The node must be in the list. Caller receives ownership of the
> + * removed node and must release it with bpf_obj_drop.
> + * Returns
> + * Pointer to the removed bpf_list_node, or NULL if 'node' is NULL
> + * or not in the list.
> + */
> +extern struct bpf_list_node *bpf_list_del(struct bpf_list_head *head,
> + struct bpf_list_node *node) __ksym;
> +
> /* Description
> * Remove 'node' from rbtree with root 'root'
> * Returns
> diff --git a/tools/testing/selftests/bpf/progs/refcounted_kptr.c
> b/tools/testing/selftests/bpf/progs/refcounted_kptr.c
> index 1aca85d86aeb..ac7672cfefb8 100644
> --- a/tools/testing/selftests/bpf/progs/refcounted_kptr.c
> +++ b/tools/testing/selftests/bpf/progs/refcounted_kptr.c
> @@ -367,6 +367,77 @@ long insert_rbtree_and_stash__del_tree_##rem_tree(void
> *ctx) \
> INSERT_STASH_READ(true, "insert_stash_read: remove from tree");
> INSERT_STASH_READ(false, "insert_stash_read: don't remove from tree");
>
> +/* Insert node_data into both rbtree and list, remove from tree, then remove
> + * from list via bpf_list_del using the node obtained from the tree.
> + */
> +SEC("tc")
> +__description("test_bpf_list_del: remove an arbitrary node from the list")
> +__success __retval(0)
> +long test_bpf_list_del(void *ctx)
> +{
> + long err;
> + struct bpf_rb_node *rb;
> + struct bpf_list_node *l;
> + struct node_data *n;
> +
> + err = __insert_in_tree_and_list(&head, &root, &lock);
> + if (err)
> + return err;
> +
> + bpf_spin_lock(&lock);
> + rb = bpf_rbtree_first(&root);
> + if (!rb) {
> + bpf_spin_unlock(&lock);
> + return -4;
> + }
> +
> + rb = bpf_rbtree_remove(&root, rb);
> + if (!rb) {
> + bpf_spin_unlock(&lock);
> + return -5;
> + }
> +
> + n = container_of(rb, struct node_data, r);
> + l = bpf_list_del(&head, &n->l);
> + bpf_spin_unlock(&lock);
> + bpf_obj_drop(n);
> + if (!l)
> + return -6;
> +
> + bpf_obj_drop(container_of(l, struct node_data, l));
> + return 0;
> +}
> +
> +SEC("?tc")
> +__failure __msg("bpf_spin_lock at off=32 must be held for bpf_list_head")
> +long list_del_without_lock_fail(void *ctx)
> +{
> + struct bpf_rb_node *rb;
> + struct bpf_list_node *l;
> + struct node_data *n;
> +
> + bpf_spin_lock(&lock);
> + rb = bpf_rbtree_first(&root);
> + if (!rb) {
> + bpf_spin_unlock(&lock);
> + return -4;
Do we need to craft those error return codes in the program that is
supposed to be rejected by verifier. Can we implement a minimal code
that triggers the verifier error without all unnecessary noise.
> + }
> +
> + rb = bpf_rbtree_remove(&root, rb);
> + bpf_spin_unlock(&lock);
> + if (!rb)
> + return -5;
> +
> + n = container_of(rb, struct node_data, r);
> + l = bpf_list_del(&head, &n->l);
> + bpf_obj_drop(n);
> + if (!l)
> + return -6;
> +
> + bpf_obj_drop(container_of(l, struct node_data, l));
> + return 0;
> +}
> +
> SEC("tc")
> __success
> long rbtree_refcounted_node_ref_escapes(void *ctx)
> --
> 2.50.1 (Apple Git-155)
