On 22/01/2025 02:53, Ihor Solodrai wrote:
> BTF type tags and decl tags now may have info->kflag set to 1,
> changing the semantics of the tag.
>
> Change BTF verification to permit BTF that makes use of this feature:
> * remove kflag check in btf_decl_tag_check_meta(), as both values
> are valid
> * allow kflag to be set for BTF_KIND_TYPE_TAG type in
> btf_ref_type_check_meta()
>
> Modify a selftest checking for kflag in decl_tag accordingly.
>
> Signed-off-by: Ihor Solodrai <ihor.solodrai@xxxxx>
Looks good, but I have a few questions here. Today in btf_struct_walk()
we check pointer type tags like this:
/* check type tag */
t = btf_type_by_id(btf, mtype->type);
if (btf_type_is_type_tag(t)) {
tag_value = __btf_name_by_offset(btf,
t->name_off);
/* check __user tag */
if (strcmp(tag_value, "user") == 0)
tmp_flag = MEM_USER;
/* check __percpu tag */
if (strcmp(tag_value, "percpu") == 0)
tmp_flag = MEM_PERCPU;
/* check __rcu tag */
if (strcmp(tag_value, "rcu") == 0)
tmp_flag = MEM_RCU;
}
Do we need to add in a check for kind_flag == 0 here to ensure it's a
BTF type tag attribute? Similar question for type tag kptr checking in
btf_find_kptr() (we could perhaps add a btf_type_is_btf_type_tag() or
something to include the kind_flag == 0 check).
And I presume the btf_check_type_tags() logic still applies for generic
attributes - i.e. that they always precede the modifiers in the chain?
> ---
> kernel/bpf/btf.c | 7 +------
> tools/testing/selftests/bpf/prog_tests/btf.c | 4 +---
> 2 files changed, 2 insertions(+), 9 deletions(-)
>
> diff --git a/kernel/bpf/btf.c b/kernel/bpf/btf.c
> index 8396ce1d0fba..becdec583e00 100644
> --- a/kernel/bpf/btf.c
> +++ b/kernel/bpf/btf.c
> @@ -2575,7 +2575,7 @@ static int btf_ref_type_check_meta(struct btf_verifier_env *env,
> return -EINVAL;
> }
>
> - if (btf_type_kflag(t)) {
> + if (btf_type_kflag(t) && BTF_INFO_KIND(t->info) != BTF_KIND_TYPE_TAG) {
> btf_verifier_log_type(env, t, "Invalid btf_info kind_flag");
> return -EINVAL;
> }
> @@ -4944,11 +4944,6 @@ static s32 btf_decl_tag_check_meta(struct btf_verifier_env *env,
> return -EINVAL;
> }
>
> - if (btf_type_kflag(t)) {
> - btf_verifier_log_type(env, t, "Invalid btf_info kind_flag");
> - return -EINVAL;
> - }
> -
> component_idx = btf_type_decl_tag(t)->component_idx;
> if (component_idx < -1) {
> btf_verifier_log_type(env, t, "Invalid component_idx");
> diff --git a/tools/testing/selftests/bpf/prog_tests/btf.c b/tools/testing/selftests/bpf/prog_tests/btf.c
> index e63d74ce046f..aab9ad88c845 100644
> --- a/tools/testing/selftests/bpf/prog_tests/btf.c
> +++ b/tools/testing/selftests/bpf/prog_tests/btf.c
> @@ -3866,7 +3866,7 @@ static struct btf_raw_test raw_tests[] = {
> .err_str = "vlen != 0",
> },
> {
> - .descr = "decl_tag test #8, invalid kflag",
> + .descr = "decl_tag test #8, tag with kflag",
> .raw_types = {
> BTF_TYPE_INT_ENC(0, BTF_INT_SIGNED, 0, 32, 4), /* [1] */
> BTF_VAR_ENC(NAME_TBD, 1, 0), /* [2] */
> @@ -3881,8 +3881,6 @@ static struct btf_raw_test raw_tests[] = {
> .key_type_id = 1,
> .value_type_id = 1,
> .max_entries = 1,
> - .btf_load_err = true,
> - .err_str = "Invalid btf_info kind_flag",
> },
> {
> .descr = "decl_tag test #9, var, invalid component_idx",
