Commit 4d585f48 authored by Dave Marchevsky's avatar Dave Marchevsky Committed by Alexei Starovoitov

bpf: Remove anonymous union in bpf_kfunc_call_arg_meta

For kfuncs like bpf_obj_drop and bpf_refcount_acquire - which take
user-defined types as input - the verifier needs to track the specific
type passed in when checking a particular kfunc call. This requires
tracking (btf, btf_id) tuple. In commit 7c50b1cb
("bpf: Add bpf_refcount_acquire kfunc") I added an anonymous union with
inner structs named after the specific kfuncs tracking this information,
with the goal of making it more obvious which kfunc this data was being
tracked / expected to be tracked on behalf of.

In a recent series adding a new user of this tuple, Alexei mentioned
that he didn't like this union usage as it doesn't really help with
readability or bug-proofing ([0]). In an offline convo we agreed to
have the tuple be fields (arg_btf, arg_btf_id), with comments in
bpf_kfunc_call_arg_meta definition enumerating the uses of the fields by
kfunc-specific handling logic. Such a pattern is used by struct
bpf_reg_state without trouble.

Accordingly, this patch removes the anonymous union in favor of arg_btf
and arg_btf_id fields and comment enumerating their current uses. The
patch also removes struct btf_and_id, which was only being used by the
removed union's inner structs.

This is a mechanical change, existing linked_list and rbtree tests will
validate that correct (btf, btf_id) are being passed.

  [0]: https://lore.kernel.org/bpf/20230505021707.vlyiwy57vwxglbka@dhcp-172-26-102-232.dhcp.thefacebook.comSigned-off-by: default avatarDave Marchevsky <davemarchevsky@fb.com>
Link: https://lore.kernel.org/r/20230510213047.1633612-1-davemarchevsky@fb.comSigned-off-by: default avatarAlexei Starovoitov <ast@kernel.org>
parent 79b3604d
...@@ -279,11 +279,6 @@ struct bpf_call_arg_meta { ...@@ -279,11 +279,6 @@ struct bpf_call_arg_meta {
struct btf_field *kptr_field; struct btf_field *kptr_field;
}; };
struct btf_and_id {
struct btf *btf;
u32 btf_id;
};
struct bpf_kfunc_call_arg_meta { struct bpf_kfunc_call_arg_meta {
/* In parameters */ /* In parameters */
struct btf *btf; struct btf *btf;
...@@ -302,10 +297,18 @@ struct bpf_kfunc_call_arg_meta { ...@@ -302,10 +297,18 @@ struct bpf_kfunc_call_arg_meta {
u64 value; u64 value;
bool found; bool found;
} arg_constant; } arg_constant;
union {
struct btf_and_id arg_obj_drop; /* arg_btf and arg_btf_id are used by kfunc-specific handling,
struct btf_and_id arg_refcount_acquire; * generally to pass info about user-defined local kptr types to later
}; * verification logic
* bpf_obj_drop
* Record the local kptr type to be drop'd
* bpf_refcount_acquire (via KF_ARG_PTR_TO_REFCOUNTED_KPTR arg type)
* Record the local kptr type to be refcount_incr'd
*/
struct btf *arg_btf;
u32 arg_btf_id;
struct { struct {
struct btf_field *field; struct btf_field *field;
} arg_list_head; } arg_list_head;
...@@ -10680,8 +10683,8 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_kfunc_call_ ...@@ -10680,8 +10683,8 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_kfunc_call_
} }
if (meta->btf == btf_vmlinux && if (meta->btf == btf_vmlinux &&
meta->func_id == special_kfunc_list[KF_bpf_obj_drop_impl]) { meta->func_id == special_kfunc_list[KF_bpf_obj_drop_impl]) {
meta->arg_obj_drop.btf = reg->btf; meta->arg_btf = reg->btf;
meta->arg_obj_drop.btf_id = reg->btf_id; meta->arg_btf_id = reg->btf_id;
} }
break; break;
case KF_ARG_PTR_TO_DYNPTR: case KF_ARG_PTR_TO_DYNPTR:
...@@ -10892,8 +10895,8 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_kfunc_call_ ...@@ -10892,8 +10895,8 @@ static int check_kfunc_args(struct bpf_verifier_env *env, struct bpf_kfunc_call_
verbose(env, "bpf_refcount_acquire calls are disabled for now\n"); verbose(env, "bpf_refcount_acquire calls are disabled for now\n");
return -EINVAL; return -EINVAL;
} }
meta->arg_refcount_acquire.btf = reg->btf; meta->arg_btf = reg->btf;
meta->arg_refcount_acquire.btf_id = reg->btf_id; meta->arg_btf_id = reg->btf_id;
break; break;
} }
} }
...@@ -11125,12 +11128,12 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn, ...@@ -11125,12 +11128,12 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
} else if (meta.func_id == special_kfunc_list[KF_bpf_refcount_acquire_impl]) { } else if (meta.func_id == special_kfunc_list[KF_bpf_refcount_acquire_impl]) {
mark_reg_known_zero(env, regs, BPF_REG_0); mark_reg_known_zero(env, regs, BPF_REG_0);
regs[BPF_REG_0].type = PTR_TO_BTF_ID | MEM_ALLOC; regs[BPF_REG_0].type = PTR_TO_BTF_ID | MEM_ALLOC;
regs[BPF_REG_0].btf = meta.arg_refcount_acquire.btf; regs[BPF_REG_0].btf = meta.arg_btf;
regs[BPF_REG_0].btf_id = meta.arg_refcount_acquire.btf_id; regs[BPF_REG_0].btf_id = meta.arg_btf_id;
insn_aux->kptr_struct_meta = insn_aux->kptr_struct_meta =
btf_find_struct_meta(meta.arg_refcount_acquire.btf, btf_find_struct_meta(meta.arg_btf,
meta.arg_refcount_acquire.btf_id); meta.arg_btf_id);
} else if (meta.func_id == special_kfunc_list[KF_bpf_list_pop_front] || } else if (meta.func_id == special_kfunc_list[KF_bpf_list_pop_front] ||
meta.func_id == special_kfunc_list[KF_bpf_list_pop_back]) { meta.func_id == special_kfunc_list[KF_bpf_list_pop_back]) {
struct btf_field *field = meta.arg_list_head.field; struct btf_field *field = meta.arg_list_head.field;
...@@ -11260,8 +11263,8 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn, ...@@ -11260,8 +11263,8 @@ static int check_kfunc_call(struct bpf_verifier_env *env, struct bpf_insn *insn,
if (meta.btf == btf_vmlinux && btf_id_set_contains(&special_kfunc_set, meta.func_id)) { 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]) { if (meta.func_id == special_kfunc_list[KF_bpf_obj_drop_impl]) {
insn_aux->kptr_struct_meta = insn_aux->kptr_struct_meta =
btf_find_struct_meta(meta.arg_obj_drop.btf, btf_find_struct_meta(meta.arg_btf,
meta.arg_obj_drop.btf_id); meta.arg_btf_id);
} }
} }
} }
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment