On Sat, Dec 17, 2022 at 12:25:00AM -0800, Dave Marchevsky wrote:
>
> +static int
> +__process_kf_arg_ptr_to_graph_node(struct bpf_verifier_env *env,
> + struct bpf_reg_state *reg, u32 regno,
> + struct bpf_kfunc_call_arg_meta *meta,
> + enum btf_field_type head_field_type,
> + enum btf_field_type node_field_type,
> + struct btf_field **node_field)
> +{
> + const char *node_type_name;
> const struct btf_type *et, *t;
> struct btf_field *field;
> - u32 list_node_off;
> + u32 node_off;
>
> - if (meta->btf != btf_vmlinux ||
> - (meta->func_id != special_kfunc_list[KF_bpf_list_push_front] &&
> - meta->func_id != special_kfunc_list[KF_bpf_list_push_back])) {
> - verbose(env, "verifier internal error: bpf_list_node argument for unknown kfunc\n");
> + if (meta->btf != btf_vmlinux) {
> + verbose(env, "verifier internal error: unexpected btf mismatch in kfunc call\n");
> return -EFAULT;
> }
>
> + if (!check_kfunc_is_graph_node_api(env, node_field_type, meta->func_id))
> + return -EFAULT;
> +
> + node_type_name = btf_field_type_name(node_field_type);
> if (!tnum_is_const(reg->var_off)) {
> verbose(env,
> - "R%d doesn't have constant offset. bpf_list_node has to be at the constant offset\n",
> - regno);
> + "R%d doesn't have constant offset. %s has to be at the constant offset\n",
> + regno, node_type_name);
> return -EINVAL;
> }
>
> - list_node_off = reg->off + reg->var_off.value;
> - field = reg_find_field_offset(reg, list_node_off, BPF_LIST_NODE);
> - if (!field || field->offset != list_node_off) {
> - verbose(env, "bpf_list_node not found at offset=%u\n", list_node_off);
> + node_off = reg->off + reg->var_off.value;
> + field = reg_find_field_offset(reg, node_off, node_field_type);
> + if (!field || field->offset != node_off) {
> + verbose(env, "%s not found at offset=%u\n", node_type_name, node_off);
> return -EINVAL;
> }
>
> - field = meta->arg_list_head.field;
> + field = *node_field;
>
> et = btf_type_by_id(field->graph_root.btf, field->graph_root.value_btf_id);
> t = btf_type_by_id(reg->btf, reg->btf_id);
> if (!btf_struct_ids_match(&env->log, reg->btf, reg->btf_id, 0, field->graph_root.btf,
> field->graph_root.value_btf_id, true)) {
> - verbose(env, "operation on bpf_list_head expects arg#1 bpf_list_node at offset=%d "
> + verbose(env, "operation on %s expects arg#1 %s at offset=%d "
> "in struct %s, but arg is at offset=%d in struct %s\n",
> + btf_field_type_name(head_field_type),
> + btf_field_type_name(node_field_type),
> field->graph_root.node_offset,
> btf_name_by_offset(field->graph_root.btf, et->name_off),
> - list_node_off, btf_name_by_offset(reg->btf, t->name_off));
> + node_off, btf_name_by_offset(reg->btf, t->name_off));
> return -EINVAL;
> }
>
> - if (list_node_off != field->graph_root.node_offset) {
> - verbose(env, "arg#1 offset=%d, but expected bpf_list_node at offset=%d in struct %s\n",
> - list_node_off, field->graph_root.node_offset,
> + if (node_off != field->graph_root.node_offset) {
> + verbose(env, "arg#1 offset=%d, but expected %s at offset=%d in struct %s\n",
> + node_off, btf_field_type_name(node_field_type),
> + field->graph_root.node_offset,
> btf_name_by_offset(field->graph_root.btf, et->name_off));
> return -EINVAL;
> }
> @@ -8932,6 +9053,24 @@ static int process_kf_arg_ptr_to_list_node(struct bpf_verifier_env *env,
> return 0;
> }
and with suggestion in patch 2 the single __process_kf_arg_ptr_to_graph_node helper
called as:
> +static int process_kf_arg_ptr_to_list_node(struct bpf_verifier_env *env,
> + struct bpf_reg_state *reg, u32 regno,
> + struct bpf_kfunc_call_arg_meta *meta)
> +{
> + return __process_kf_arg_ptr_to_graph_node(env, reg, regno, meta,
> + BPF_LIST_HEAD, BPF_LIST_NODE,
> + &meta->arg_list_head.field);
> +}
> +
> +static int process_kf_arg_ptr_to_rbtree_node(struct bpf_verifier_env *env,
> + struct bpf_reg_state *reg, u32 regno,
> + struct bpf_kfunc_call_arg_meta *meta)
> +{
> + return __process_kf_arg_ptr_to_graph_node(env, reg, regno, meta,
> + BPF_RB_ROOT, BPF_RB_NODE,
> + &meta->arg_rbtree_root.field);
> +}
would convert the arg from owning to non-owning.
