| Message ID | 20181127051715.1845528-2-yhs@fb.com |
|---|---|
| State | Changes Requested, archived |
| Delegated to: | BPF Maintainers |
| Headers | show |
| Series | bpf: btf: check name validity for various types | expand |
On Mon, Nov 26, 2018 at 09:17:13PM -0800, Yonghong Song wrote: > Commit 2667a2626f4d ("bpf: btf: Add BTF_KIND_FUNC > and BTF_KIND_FUNC_PROTO") checked the name validity > for BTF_KIND_FUNC/BTF_KIND_FUNC_PROTO types such that: > . BTF_KIND_FUNC must have a valid identifier name > . BTF_KIND_PROTO must have a null name > . The argument name of BTF_KIND_FUNC/BTF_KIND_FUNC_PROTO, > if not null, must be a valid identifier. > > This patch added name checking for the following types: > . BTF_KIND_PTR, BTF_KIND_ARRAY, BTF_KIND_VOLATILE, > BTF_KIND_CONST, BTF_KIND_RESTRICT: > the name must be null > . BTF_KIND_STRUCT, BTF_KIND_UNION: the struct/member name > is either null or a valid identifier > . BTF_KIND_ENUM: the enum type name is either null or a valid > identifier; the enumerator name must be a valid identifier. > . BTF_KIND_FWD: the name must be a valid identifier > . BTF_KIND_TYPEDEF: the name must be a valid identifier > > For those places a valid name is required, the name must be > a valid C identifier. This can be relaxed later if we found > use cases for a different (non-C) frontend. > > Acked-by: Martin KaFai Lau <kafai@fb.com> > Signed-off-by: Yonghong Song <yhs@fb.com> ... > return 0; > @@ -1409,6 +1432,12 @@ static s32 btf_array_check_meta(struct btf_verifier_env *env, > return -EINVAL; > } > > + /* array type should not have a name */ > + if (t->name_off) { > + btf_verifier_log_type(env, t, "Invalid name"); > + return -EINVAL; > + } > + > if (btf_type_vlen(t)) { > btf_verifier_log_type(env, t, "vlen != 0"); > return -EINVAL; > @@ -1585,6 +1614,13 @@ static s32 btf_struct_check_meta(struct btf_verifier_env *env, > return -EINVAL; > } > > + /* struct type either no name or a valid one */ > + if (t->name_off && > + !btf_name_valid_identifier(env->btf, t->name_off)) { Looks like some of these changes need to go into bpf tree. please split it up and let's try to minimize the conflicts between bpf and bpf-next Thanks!
On 11/27/18 11:02 AM, Alexei Starovoitov wrote: > On Mon, Nov 26, 2018 at 09:17:13PM -0800, Yonghong Song wrote: >> Commit 2667a2626f4d ("bpf: btf: Add BTF_KIND_FUNC >> and BTF_KIND_FUNC_PROTO") checked the name validity >> for BTF_KIND_FUNC/BTF_KIND_FUNC_PROTO types such that: >> . BTF_KIND_FUNC must have a valid identifier name >> . BTF_KIND_PROTO must have a null name >> . The argument name of BTF_KIND_FUNC/BTF_KIND_FUNC_PROTO, >> if not null, must be a valid identifier. >> >> This patch added name checking for the following types: >> . BTF_KIND_PTR, BTF_KIND_ARRAY, BTF_KIND_VOLATILE, >> BTF_KIND_CONST, BTF_KIND_RESTRICT: >> the name must be null >> . BTF_KIND_STRUCT, BTF_KIND_UNION: the struct/member name >> is either null or a valid identifier >> . BTF_KIND_ENUM: the enum type name is either null or a valid >> identifier; the enumerator name must be a valid identifier. >> . BTF_KIND_FWD: the name must be a valid identifier >> . BTF_KIND_TYPEDEF: the name must be a valid identifier >> >> For those places a valid name is required, the name must be >> a valid C identifier. This can be relaxed later if we found >> use cases for a different (non-C) frontend. >> >> Acked-by: Martin KaFai Lau <kafai@fb.com> >> Signed-off-by: Yonghong Song <yhs@fb.com> > ... >> return 0; >> @@ -1409,6 +1432,12 @@ static s32 btf_array_check_meta(struct btf_verifier_env *env, >> return -EINVAL; >> } >> >> + /* array type should not have a name */ >> + if (t->name_off) { >> + btf_verifier_log_type(env, t, "Invalid name"); >> + return -EINVAL; >> + } >> + >> if (btf_type_vlen(t)) { >> btf_verifier_log_type(env, t, "vlen != 0"); >> return -EINVAL; >> @@ -1585,6 +1614,13 @@ static s32 btf_struct_check_meta(struct btf_verifier_env *env, >> return -EINVAL; >> } >> >> + /* struct type either no name or a valid one */ >> + if (t->name_off && >> + !btf_name_valid_identifier(env->btf, t->name_off)) { > > Looks like some of these changes need to go into bpf tree. > please split it up and let's try to minimize the conflicts between bpf and bpf-next Make sense. Will restructure and resubmit for bpf in the next version. > Thanks! >
diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c index a09b2f94ab25..793acba40b4c 100644 --- a/kernel/bpf/btf.c +++ b/kernel/bpf/btf.c @@ -1195,6 +1195,22 @@ static int btf_ref_type_check_meta(struct btf_verifier_env *env, return -EINVAL; } + /* typedef type must have a valid name, and other ref types, + * volatile, const, restrict, should have a null name. + */ + if (BTF_INFO_KIND(t->info) == BTF_KIND_TYPEDEF) { + if (!t->name_off || + !btf_name_valid_identifier(env->btf, t->name_off)) { + btf_verifier_log_type(env, t, "Invalid name"); + return -EINVAL; + } + } else { + if (t->name_off) { + btf_verifier_log_type(env, t, "Invalid name"); + return -EINVAL; + } + } + btf_verifier_log_type(env, t, NULL); return 0; @@ -1353,6 +1369,13 @@ static s32 btf_fwd_check_meta(struct btf_verifier_env *env, return -EINVAL; } + /* fwd type must have a valid name */ + if (!t->name_off || + !btf_name_valid_identifier(env->btf, t->name_off)) { + btf_verifier_log_type(env, t, "Invalid name"); + return -EINVAL; + } + btf_verifier_log_type(env, t, NULL); return 0; @@ -1409,6 +1432,12 @@ static s32 btf_array_check_meta(struct btf_verifier_env *env, return -EINVAL; } + /* array type should not have a name */ + if (t->name_off) { + btf_verifier_log_type(env, t, "Invalid name"); + return -EINVAL; + } + if (btf_type_vlen(t)) { btf_verifier_log_type(env, t, "vlen != 0"); return -EINVAL; @@ -1585,6 +1614,13 @@ static s32 btf_struct_check_meta(struct btf_verifier_env *env, return -EINVAL; } + /* struct type either no name or a valid one */ + if (t->name_off && + !btf_name_valid_identifier(env->btf, t->name_off)) { + btf_verifier_log_type(env, t, "Invalid name"); + return -EINVAL; + } + btf_verifier_log_type(env, t, NULL); last_offset = 0; @@ -1596,6 +1632,12 @@ static s32 btf_struct_check_meta(struct btf_verifier_env *env, return -EINVAL; } + /* struct member either no name or a valid one */ + if (member->name_off && + !btf_name_valid_identifier(btf, member->name_off)) { + btf_verifier_log_member(env, t, member, "Invalid name"); + return -EINVAL; + } /* A member cannot be in type void */ if (!member->type || !BTF_TYPE_ID_VALID(member->type)) { btf_verifier_log_member(env, t, member, @@ -1783,6 +1825,13 @@ static s32 btf_enum_check_meta(struct btf_verifier_env *env, return -EINVAL; } + /* enum type either no name or a valid one */ + if (t->name_off && + !btf_name_valid_identifier(env->btf, t->name_off)) { + btf_verifier_log_type(env, t, "Invalid name"); + return -EINVAL; + } + btf_verifier_log_type(env, t, NULL); for (i = 0; i < nr_enums; i++) { @@ -1792,6 +1841,14 @@ static s32 btf_enum_check_meta(struct btf_verifier_env *env, return -EINVAL; } + /* enum member must have a valid name */ + if (!enums[i].name_off || + !btf_name_valid_identifier(btf, enums[i].name_off)) { + btf_verifier_log_type(env, t, "Invalid name"); + return -EINVAL; + } + + btf_verifier_log(env, "\t%s val=%d\n", btf_name_by_offset(btf, enums[i].name_off), enums[i].val);
Commit 2667a2626f4d ("bpf: btf: Add BTF_KIND_FUNC and BTF_KIND_FUNC_PROTO") checked the name validity for BTF_KIND_FUNC/BTF_KIND_FUNC_PROTO types such that: . BTF_KIND_FUNC must have a valid identifier name . BTF_KIND_PROTO must have a null name . The argument name of BTF_KIND_FUNC/BTF_KIND_FUNC_PROTO, if not null, must be a valid identifier. This patch added name checking for the following types: . BTF_KIND_PTR, BTF_KIND_ARRAY, BTF_KIND_VOLATILE, BTF_KIND_CONST, BTF_KIND_RESTRICT: the name must be null . BTF_KIND_STRUCT, BTF_KIND_UNION: the struct/member name is either null or a valid identifier . BTF_KIND_ENUM: the enum type name is either null or a valid identifier; the enumerator name must be a valid identifier. . BTF_KIND_FWD: the name must be a valid identifier . BTF_KIND_TYPEDEF: the name must be a valid identifier For those places a valid name is required, the name must be a valid C identifier. This can be relaxed later if we found use cases for a different (non-C) frontend. Acked-by: Martin KaFai Lau <kafai@fb.com> Signed-off-by: Yonghong Song <yhs@fb.com> --- kernel/bpf/btf.c | 57 ++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 57 insertions(+)